benoitc / hackney

simple HTTP client in Erlang
Other
1.34k stars 427 forks source link

Newline character is not escaped in headers #506

Open SteamRiC opened 6 years ago

SteamRiC commented 6 years ago

Example code using hackney 1.12.1:

application:ensure_all_started(hackney).

Method = post,
URL = <<"http://google.com">>,
Headers = [
  {<<"Custom-Header">>, <<"Value\n">>}
],
Payload = <<>>,
Options = [],
{ok, StatusCode, RespHeaders, ClientRef} = hackney:request(Method, URL,
                                                        Headers, Payload,
                                                        Options).

When inspecting the requests in Wireshark, I got the following results:

With {<<"Custom-Header">>, <<"Value">>} (without \n at the end)

Hypertext Transfer Protocol
    POST / HTTP/1.1\r\n
    Custom-Header: Value\r\n
    Host: google.com\r\n
    User-Agent: hackney/1.12.1\r\n
    Content-Type: application/octet-stream\r\n
    Content-Length: 0\r\n
    \r\n
    [Full request URI: http://google.com/]
    [HTTP request 1/1]
    [Response in frame: 782]

With {<<"Custom-Header">>, <<"Value\n">>}

Hypertext Transfer Protocol
    POST / HTTP/1.1\r\n
    Custom-Header: Value\n
    \r\n
    [HTTP request 1/1]
    [Response in frame: 1793]

Hypertext Transfer Protocol
    Host: google.com\r\n
    User-Agent: hackney/1.12.1\r\n
    Content-Type: application/octet-stream\r\n
    Content-Length: 0\r\n
    \r\n
AlimovSV commented 6 years ago

Also one example with bad escaping:

iex>  :hackney_multipart.encode_form([{:file, "1\"2.txt"}]) |> elem(0) |> IO.puts
-----------------------------cmvkvxjlirmydmjx
content-length: 0
content-type: text/plain
content-disposition: form-data; name="file"; filename="1"2.txt"

-----------------------------cmvkvxjlirmydmjx--

filename="1"2.txt" doesn't look correct

benoitc commented 6 years ago

mmm shouldn't trailing new line should rather stripped instead of escaped? For line in the middle yes that should be encoded, but trailing new lines?

sauvainr commented 5 years ago

Facing the same issue. Any follow up on this?

It can be potentially dangerous as it allows injection through header value.

[
  {<<"Custom-Header1">>, <<"Value\nEvilHeader: evilvalue">>}
],

Line break should be followed by space or tab character. Any counter indication to parse & add space if missing?