URIs.jl icon indicating copy to clipboard operation
URIs.jl copied to clipboard

Published 20 hours ago • verified publisher icon JuliaWeb

Properly check for invalid characters when parsing URI-string

Open Seelengrab opened this issue 6 years ago • 2 comments

https://github.com/JuliaWeb/HTTP.jl/blob/668e7e68747bb333ebde13af8d16add5b82b3b8a/src/URIs.jl#L120-L132

The quoted section of code doesn't actually check whether a URI contains invalid characters or not - e.g., ' ' (space character) is not allowed in the host part of an authority (or anywhere in a URI for that matter as far as I can tell), but still makes it through and can lead to some weird requests. There's also no two @ allowed.

When parsing user-typed URIs "should attempt to recognize and strip both delimiters and embedded whitespace", according to RFC3986.

It should be noted that the regex from the RFC for seperating valid URIs doesn't check for invalid characters that are not explicitly excluded in the grammar definitions and so is not enough for ensuring a URI is valid.

Here a MWE for showing the fault:

julia> using HTTP

julia> badURL = "http://[email protected] @test.com:8080/test"

julia> t = HTTP.URIs.URI(badURL)

julia> for f in 1:fieldcount(typeof(t))
           n = fieldname(typeof(t),f)
           println("$n - '$(getfield(t, f))'")
       end
uri - 'http://[email protected] @test.com:8080/test'
scheme - 'http'
userinfo - 'foo'
host - '127.0.0.1 @test.com'
port - '8080'
path - '/test'
query - ''
fragment - ''

julia> HTTP.get(badURL)

and in a seperate terminal session:

<snip>:~ $ nc -l -p 8080

GET /test HTTP/1.1
Host: 127.0.0.1 @test.com
Content-Length: 0

That the request actually goes through is a problem with Sockets.getaddrinfo though, since that does no checking either and the underlying OS library returns 127.0.0.1 for the (invalid) host 127.0.0.1 @test.com on my machine.

julia> versioninfo()
Julia Version 1.2.0
Commit c6da87ff4b (2019-08-20 00:03 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: Intel(R) Core(TM) i7-6600U CPU @ 2.60GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-6.0.1 (ORCJIT, skylake)
Environment:
  JULIA_NUM_THREADS = 4

(v1.2) pkg> st
    Status `~/.julia/environments/v1.2/Project.toml`
  [cd3eb016] HTTP v0.8.5

Seelengrab avatar Aug 28 '19 20:08 Seelengrab

This issue was originally inspired by "A New Era of SSRF - Exploiting URL Parser in Trending Programming Languages!" and is thus security relevant. Since this package is an important dependency for many packages, this is probably a critical bug. It should be enough to properly filter for the allowed characters before retrieving the parts of the URL with the regex.

For a change, trying this today gives me the following error:

julia> HTTP.get(badURL)
ERROR: DNSError: 127.0.0.1 @test.com, unknown node or service (EAI_NONAME)                      Stacktrace:                                                                                      [1] getalladdrinfo(::String) at /home/<snip>/julia/usr/share/julia/stdlib/v1.5/Sockets/src/addrinfo.jl:112
 [2] getalladdrinfo at /home/<snip>/julia/usr/share/julia/stdlib/v1.5/Sockets/src/addrinfo.jl:121 [inlined]

which bubbles up from getalladdrinfo from the Sockets stdlib as a DNSError, but this should be caught much sooner and not even trigger a request to DNS since this is neither a valid URL nor has a valid host in the first place. Adding on to that, I don't know if that error is OS specific or not, different OS libraries might handle those bad hosts differently and actually let the request through, as it was the case when this issue was created.

Seelengrab avatar Jun 09 '20 05:06 Seelengrab

Trying to reproduce this now leads to a DNSError because the OS layer on my machine has been fixed:

error output of `HTTP.get(bad_url)` 
ERROR: HTTP.ConnectError for url = `http://[email protected] @test.com:8080/test`: DNSError: 127.0.0.1 @test.com, unknown node or service (EAI_NONAME)
Stacktrace:
 [1] getalladdrinfo(host::String)
   @ Sockets ~/julia/usr/share/julia/stdlib/v1.12/Sockets/src/addrinfo.jl:113
 [2] getalladdrinfo
   @ ~/julia/usr/share/julia/stdlib/v1.12/Sockets/src/addrinfo.jl:122 [inlined]
 [3] getconnection(::Type{…}, host::SubString{…}, port::SubString{…}; keepalive::Bool, readtimeout::Int64, kw::@Kwargs{…})
   @ HTTP.Connections ~/.julia/packages/HTTP/enKbm/src/Connections.jl:515
 [4] getconnection
   @ ~/.julia/packages/HTTP/enKbm/src/Connections.jl:503 [inlined]
 [5] #10
   @ ~/.julia/packages/HTTP/enKbm/src/Connections.jl:462 [inlined]
 [6] macro expansion
   @ ~/.julia/packages/ConcurrentUtilities/J6iMP/src/try_with_timeout.jl:82 [inlined]
 [7] (::ConcurrentUtilities.var"#2#4"{…})()
   @ ConcurrentUtilities ~/.julia/packages/ConcurrentUtilities/J6iMP/src/ConcurrentUtilities.jl:9
Stacktrace:
  [1] (::HTTP.ConnectionRequest.var"#connections#4"{…})(req::HTTP.Messages.Request; proxy::Nothing, socket_type::Type, socket_type_tls::Type, readtimeout::Int64, connect_timeout::Int64, logerrors::Bool, logtag::Nothing, kw::@Kwargs{…})
    @ HTTP.ConnectionRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/ConnectionRequest.jl:64
  [2] (::Base.var"#106#108"{…})(args::HTTP.Messages.Request; kwargs::@Kwargs{…})
    @ Base ./error.jl:298
  [3] (::HTTP.RetryRequest.var"#manageretries#3"{…})(req::HTTP.Messages.Request; retry::Bool, retries::Int64, retry_delays::ExponentialBackOff, retry_check::Function, retry_non_idempotent::Bool, kw::@Kwargs{…})
    @ HTTP.RetryRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/RetryRequest.jl:75
  [4] manageretries
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/RetryRequest.jl:30 [inlined]
  [5] (::HTTP.CookieRequest.var"#managecookies#4"{…})(req::HTTP.Messages.Request; cookies::Bool, cookiejar::HTTP.Cookies.CookieJar, kw::@Kwargs{…})
    @ HTTP.CookieRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/CookieRequest.jl:42
  [6] managecookies
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/CookieRequest.jl:19 [inlined]
  [7] macro expansion
    @ ./logging.jl:143 [inlined]
  [8] (::HTTP.HeadersRequest.var"#defaultheaders#2"{…})(req::HTTP.Messages.Request; iofunction::Nothing, decompress::Nothing, basicauth::Bool, detect_content_type::Bool, canonicalize_headers::Bool, kw::@Kwargs{…})
    @ HTTP.HeadersRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/HeadersRequest.jl:32
  [9] defaultheaders
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/HeadersRequest.jl:14 [inlined]
 [10] (::HTTP.RedirectRequest.var"#redirects#3"{…})(req::HTTP.Messages.Request; redirect::Bool, redirect_limit::Int64, redirect_method::Nothing, forwardheaders::Bool, response_stream::Nothing, kw::@Kwargs{…})
    @ HTTP.RedirectRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/RedirectRequest.jl:25
 [11] redirects
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/RedirectRequest.jl:14 [inlined]
 [12] (::HTTP.MessageRequest.var"#makerequest#3"{…})(method::String, url::URIs.URI, headers::Nothing, body::Vector{…}; copyheaders::Bool, response_stream::Nothing, http_version::HTTP.Strings.HTTPVersion, verbose::Int64, kw::@Kwargs{})
    @ HTTP.MessageRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/MessageRequest.jl:35
 [13] makerequest
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/MessageRequest.jl:24 [inlined]
 [14] request(stack::HTTP.MessageRequest.var"#makerequest#3"{…}, method::String, url::String, h::Nothing, b::Vector{…}, q::Nothing; headers::Nothing, body::Vector{…}, query::Nothing, kw::@Kwargs{})
    @ HTTP ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:457
 [15] request(stack::Function, method::String, url::String, h::Nothing, b::Vector{UInt8}, q::Nothing)
    @ HTTP ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:455
 [16] #request#20
    @ ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:315 [inlined]
 [17] request (repeats 2 times)
    @ ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:313 [inlined]
 [18] get(a::String)
    @ HTTP ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:518
 [19] top-level scope
    @ REPL[7]:1

caused by: DNSError: 127.0.0.1 @test.com, unknown node or service (EAI_NONAME)
Stacktrace:
 [1] getalladdrinfo(host::String)
   @ Sockets ~/julia/usr/share/julia/stdlib/v1.12/Sockets/src/addrinfo.jl:113
 [2] getalladdrinfo
   @ ~/julia/usr/share/julia/stdlib/v1.12/Sockets/src/addrinfo.jl:122 [inlined]
 [3] getconnection(::Type{…}, host::SubString{…}, port::SubString{…}; keepalive::Bool, readtimeout::Int64, kw::@Kwargs{…})
   @ HTTP.Connections ~/.julia/packages/HTTP/enKbm/src/Connections.jl:515
 [4] getconnection
   @ ~/.julia/packages/HTTP/enKbm/src/Connections.jl:503 [inlined]
 [5] #10
   @ ~/.julia/packages/HTTP/enKbm/src/Connections.jl:462 [inlined]
 [6] macro expansion
   @ ~/.julia/packages/ConcurrentUtilities/J6iMP/src/try_with_timeout.jl:82 [inlined]
 [7] (::ConcurrentUtilities.var"#2#4"{…})()
   @ ConcurrentUtilities ~/.julia/packages/ConcurrentUtilities/J6iMP/src/ConcurrentUtilities.jl:9
Stacktrace:
  [1] try_yieldto(undo::typeof(Base.ensure_rescheduled))
    @ Base ./task.jl:1085
  [2] wait()
    @ Base ./task.jl:1149
  [3] wait(c::Base.GenericCondition{ReentrantLock}; first::Bool)
    @ Base ./condition.jl:132
  [4] wait
    @ ./condition.jl:127 [inlined]
  [5] take_unbuffered(c::Channel{Any})
    @ Base ./channels.jl:510
  [6] take!
    @ ./channels.jl:487 [inlined]
  [7] try_with_timeout(f::Function, timeout::Int64, ::Type{Any})
    @ ConcurrentUtilities ~/.julia/packages/ConcurrentUtilities/J6iMP/src/try_with_timeout.jl:89
  [8] try_with_timeout
    @ ~/.julia/packages/ConcurrentUtilities/J6iMP/src/try_with_timeout.jl:77 [inlined]
  [9] (::HTTP.Connections.var"#9#12"{Sockets.TCPSocket, Int64, Int64, Bool, Bool, @Kwargs{…}, SubString{…}, SubString{…}})()
    @ HTTP.Connections ~/.julia/packages/HTTP/enKbm/src/Connections.jl:459
 [10] acquire(f::HTTP.Connections.var"#9#12"{…}, pool::ConcurrentUtilities.Pools.Pool{…}, key::Tuple{…}; forcenew::Bool, isvalid::HTTP.Connections.var"#11#14"{…})
    @ ConcurrentUtilities.Pools ~/.julia/packages/ConcurrentUtilities/J6iMP/src/pools.jl:159
 [11] acquire
    @ ~/.julia/packages/ConcurrentUtilities/J6iMP/src/pools.jl:140 [inlined]
 [12] #newconnection#8
    @ ~/.julia/packages/HTTP/enKbm/src/Connections.jl:454 [inlined]
 [13] (::HTTP.ConnectionRequest.var"#connections#4"{…})(req::HTTP.Messages.Request; proxy::Nothing, socket_type::Type, socket_type_tls::Type, readtimeout::Int64, connect_timeout::Int64, logerrors::Bool, logtag::Nothing, kw::@Kwargs{…})
    @ HTTP.ConnectionRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/ConnectionRequest.jl:80
 [14] (::Base.var"#106#108"{…})(args::HTTP.Messages.Request; kwargs::@Kwargs{…})
    @ Base ./error.jl:298
 [15] (::HTTP.RetryRequest.var"#manageretries#3"{…})(req::HTTP.Messages.Request; retry::Bool, retries::Int64, retry_delays::ExponentialBackOff, retry_check::Function, retry_non_idempotent::Bool, kw::@Kwargs{…})
    @ HTTP.RetryRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/RetryRequest.jl:75
 [16] manageretries
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/RetryRequest.jl:30 [inlined]
 [17] (::HTTP.CookieRequest.var"#managecookies#4"{…})(req::HTTP.Messages.Request; cookies::Bool, cookiejar::HTTP.Cookies.CookieJar, kw::@Kwargs{…})
    @ HTTP.CookieRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/CookieRequest.jl:42
 [18] managecookies
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/CookieRequest.jl:19 [inlined]
 [19] macro expansion
    @ ./logging.jl:143 [inlined]
 [20] (::HTTP.HeadersRequest.var"#defaultheaders#2"{…})(req::HTTP.Messages.Request; iofunction::Nothing, decompress::Nothing, basicauth::Bool, detect_content_type::Bool, canonicalize_headers::Bool, kw::@Kwargs{…})
    @ HTTP.HeadersRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/HeadersRequest.jl:32
 [21] defaultheaders
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/HeadersRequest.jl:14 [inlined]
 [22] (::HTTP.RedirectRequest.var"#redirects#3"{…})(req::HTTP.Messages.Request; redirect::Bool, redirect_limit::Int64, redirect_method::Nothing, forwardheaders::Bool, response_stream::Nothing, kw::@Kwargs{…})
    @ HTTP.RedirectRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/RedirectRequest.jl:25
 [23] redirects
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/RedirectRequest.jl:14 [inlined]
 [24] (::HTTP.MessageRequest.var"#makerequest#3"{…})(method::String, url::URIs.URI, headers::Nothing, body::Vector{…}; copyheaders::Bool, response_stream::Nothing, http_version::HTTP.Strings.HTTPVersion, verbose::Int64, kw::@Kwargs{})
    @ HTTP.MessageRequest ~/.julia/packages/HTTP/enKbm/src/clientlayers/MessageRequest.jl:35
 [25] makerequest
    @ ~/.julia/packages/HTTP/enKbm/src/clientlayers/MessageRequest.jl:24 [inlined]
 [26] request(stack::HTTP.MessageRequest.var"#makerequest#3"{…}, method::String, url::String, h::Nothing, b::Vector{…}, q::Nothing; headers::Nothing, body::Vector{…}, query::Nothing, kw::@Kwargs{})
    @ HTTP ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:457
 [27] request(stack::Function, method::String, url::String, h::Nothing, b::Vector{UInt8}, q::Nothing)
    @ HTTP ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:455
 [28] #request#20
    @ ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:315 [inlined]
 [29] request (repeats 2 times)
    @ ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:313 [inlined]
 [30] get(a::String)
    @ HTTP ~/.julia/packages/HTTP/enKbm/src/HTTP.jl:518
 [31] top-level scope
    @ REPL[7]:1
Some type information was truncated. Use `show(err)` to see complete types.

However, this is not a good error to receive here and URIs.jl should still catch this much earlier. The reasoning is that if this is run on a machine with vulnerable getaddrinfo, URIs.jl should still protect its user.

Seelengrab avatar Mar 23 '24 17:03 Seelengrab