Fix header validation

[MaeIsBad]

This PR fixes 2 issues with headers:

• Lack of any validation on the header key, leading to http header injection
• Allowing null bytes in http headers, which get rejected by most clients(libcurl, chromium, firefox)

[josevalim]

Thanks @MaeIsBad! This PR renames validate_header_key_if_test!, but it looks like it is not necessary as part of this PR. Can you please revert these particular changes and also the renaming of valid_header_key?? Thank you.

[MaeIsBad]

I can do that, but I think that it's important to differentiate between normalization of a header(all lowercase) and validation of a header(no \r\n, :, nullbytes). I can revert the change if you insist but I would prefer to keep it as is, or rename it to something else, that uses a different word from validate, if possible.

I can also make it a separate PR if that works for you

[josevalim]

💚 💙 💜 💛 ❤️

[josevalim]

@MaeIsBad btw, I understand why we validate the header value... but isn't the header key validation unnecessary? If you allow anyone to set the header, you are going to potentially have a bad time anyway (including allowing keys to be overridden), so if you are generating headers from user input, you MUST filter it through an allowed list anyway?

[MaeIsBad]

That's an interesting point I didn't even think about, I would assume that just prepending a user-controlled header with "x-user-controlled-header" or something similar would be enough to ensure the user can't do anything malicious.

Additionally if you can inject \r\n\r\n into the header you can begin to put things in the request body.

Mix.install([:plug, :plug_cowboy])

defmodule MyPlug do
  import Plug.Conn

  def init(options) do
    # initialize options
    options
  end

  def call(conn, _opts) do
    user_input = "abc: \r\n\r\n <script> alert(0); </script>"
    user_header = "x-user-header-#{user_input}"
    conn
    |> put_resp_content_type("text/html")
    |> put_resp_header(user_header, "1")
    |> send_resp(200, "The browser will read at most content-length bytes so this needs to be a bit longer")
  end
end

require Logger
{:ok, _} = Plug.Cowboy.http(MyPlug, [])
Logger.info("Plug now running on localhost:4000")

[josevalim]

Good point about x-user- prefix or similar.