Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Properly check for invalid characters when parsing URI-string #25

Open
Seelengrab opened this issue Aug 28, 2019 · 2 comments
Open

Properly check for invalid characters when parsing URI-string #25

Seelengrab opened this issue Aug 28, 2019 · 2 comments

Comments

@Seelengrab
Copy link

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
Copy link
Author

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.

@fredrikekre fredrikekre transferred this issue from JuliaWeb/HTTP.jl Apr 27, 2021
@Seelengrab
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@Seelengrab