Sunday, January 19, 2014

Header injection in Sinatra/Rack

Try to run this simple app:

require 'sinatra'
get '/' do
  redirect params[:to] if params[:to].start_with? 'http://host.com/'
end

Let's load /?to=http://host.com/?%0dX-Header:1 and see a new "injected" X-Header in Chrome (not in FF) because %0d aka \r is considered by Chrome as a valid headers' delimiter (don't really agree with this feature). OK, bad news are: Rack is the root of the problem. It uses \n internally as a delimiter for "arrays of cookies" so it blocks \n-based injections, but \r-based are working fine.

This means all web ruby software relying on Rack headers validation is vulnerable to header injection. Technically even Rails, they have "monkey patch" removing \0\r\n from "Location" header, but the rest of headers stay untouched.

Timeline
Reported to rkh from sinatra on 5 Jan. Under investigation, proposed fix was Rack Protection module.
Reported to Rails (not filtering non-Location headers), WontFix.

Let's talk about header injection
Now it is not useful, IMO. Yes you can create some new headers and such, but what can you do with it finally? Set-Cookie to mess with session_id/csrf_token is only option, am I right?

Overall it is a low-severity issue which can only insert new cookies. When browser sees non-empty Location it ignores all other headers but Set-Cookie. All you can do is BOMBIN'

5 comments:

  1. Lol, the same bug was in PHP header() function.
    Also, found such a CR-injection in some web-apps (for example, yandex mail).
    Probably, some of them were in ruby.

    ReplyDelete
  2. http://rack.rubyforge.org/doc/SPEC.html under "The Response" under "The Headers"

    I will consider altering the SPEC here to be more safe, but I could have done with a notification ahead of time. We've been through this before homakov, and you're taking my holiday away from me AGAIN through not getting in contact.

    ReplyDelete
    Replies
    1. I contacted rkh and asked him if he can do communication with you because he is closer to team and I don't even have your email.

      There was low enthusiasm to fix this from rails team too. They fixed it in Location but didn't fix Rack - for some reason? A low-severity bug is easier to publish, and it's better than arguing over email about it.

      It's 90% bug of Sinatra then, they should sanitize Location as Rails does. I don't think rack is really involved.

      Delete
    2. I will be fixing this in the handlers to prevent leakage for all headers. You missed some other good strategies here that use the same fundamental attack on the commonly unchecked aspect of the rack SPEC. They all need fixing, some are potentially more serious, like being able to turn on CORS for an endpoint. Unfortunately, I'm having trouble scheduling enough time to roll my 6 releases, which means this is a known public vector for some time.

      My only hope is that peoples frontends filter this bad behavior, and that app issues are hard to find. I'm sure that's insufficient for many people, and that's sad.

      Delete