HTTP::Server Fiber crash with "Header content contains invalid character"

[bararchy]

I have found that using some "invalid character", it's easy to crash a fiber at the server side using a simple get request.

Unhandled exception on HTTP::Handler
Header content contains invalid character '\u{6}' (ArgumentError)
Unhandled exception on HTTP::Handler
Header content contains invalid character '\u{6}' (ArgumentError)
Unhandled exception on HTTP::Handler
Header content contains invalid character '\u{6}' (ArgumentError)
Unhandled exception on HTTP::Handler
Header content contains invalid character '\u{10}' (ArgumentError)

There should be some defense against this kind of abuse especially when HTTP::Server is used as the basis for all major web frameworks and crystal play.

[asterite]

@bararchy This should have been fixed in #5041

I'm checking the code, when the request headers are parsed from the IO the add? method is used so this should be working.

What Crystal version are you using?

[bararchy]

0.24.1, this is by running crystal play

[asterite]

We probably need some code to reproduce, otherwise there's not much we can do.

[RX14]

So, does it actually crash the server or just break that specific request?

[bararchy]

The Fiber that is spawned to handle this request "breaks" with unhandled exception, this would have crash the whole server if it wouldn't have been saved by the Fiber, so Fiber dies, server is saved, but this is hardly a good solution, more of a lucky save.

I'm working on the exact payload to send to the server to cause this

[bararchy]

@asterite
This URL: http://localhost:8080/%2Fabout%2F%0A%C3%92%C3%92%C3%92%C3%92%1E against a running crystal play instance will produce the error:

Unhandled exception on HTTP::Handler
Header content contains invalid character '\n' (ArgumentError)
Unhandled exception on HTTP::Handler
Header content contains invalid character '\n' (ArgumentError)

Let me know if you need more info to tackle this

[bararchy]

@asterite @RX14 Did you manage to see this using the above method ?

[asterite]

No, I don't have time and probably won't have the time to investigate this. Someone else will have to take a look.

[straight-shoota]

Reduced code to reproduce:

require "http/server"

processor = HTTP::Server::RequestProcessor.new(HTTP::StaticFileHandler.new("."))
request = HTTP::Request.new("GET", File.basename(__FILE__) + "%0A")

io = IO::Memory.new
request.to_io(io)
io.rewind
processor.process(io, IO::Memory.new)

I'm checking the code, when the request headers are parsed from the IO the add? method is used so this should be working.

This error is caused by StaticFileHandler#redirect_to which doesn't use add?. I guess we should just extend the validity check for request_path in StaticFileHandler#call beyond checking for null char.

[straight-shoota]

A fix for the immediate issue is in #5628 but the error handling regarding fibers probably needs further investigation.

[asterite]

@straight-shoota what do you think can be improved regarding unhandled exceptions in fibers?

[straight-shoota]

I don't know.