attack against user session with omniauth.origin

[97jaz]

Please complete all sections.

Configuration

• Provider Gem: omniauth-auth0
• Ruby Version: 2.4.2
• Framework: rails-5.1.4
• Platform: OS X, Linux

Expected Behavior

Omniauth shouldn't allow untrusted parties to populate the user session with arbitrary data by blindly trusting the contents of the origin query parameter in a GET to /auth/<provider>.

Actual Behavior

Omniauth puts the contents of the origin query parameter into the user's session.

If the application is using cookies for session storage, it is trivially easy to overflow a user's session cookie by redirecting them to /auth/<provider>?origin=<string longer than 4k>. (Or, use a value that gets the session cookie close to, but not over, the 4k limit, so that auth succeeds, but any attempt to put anything into the user's session after that causes it to overflow.)

If the application is using different session storage, it opens a possible DOS attack against the storage layer.

Steps to Reproduce

Navigate to /auth/<provider>?origin=<long string>.

--

Maybe I've missed some important aspect of how this feature works, but it seems to me that saving the user's location is much easier to do securely in the application layer, rather than the middleware layer.

[tmilewski]

Unless there's something that I'm missing, this check is typically handled through Rack::Session::Cookie or, as in your case, ActionDispatch::Cookies::CookieJar.

Rack::Session::Cookie:

• https://github.com/rack/rack/blob/master/lib/rack/session/cookie.rb#L156-L170

ActionDispatch::Cookies::CookieJar

• https://github.com/rails/rails/blob/1fa268bfa5667a0e9ddbfda243b5282c023ab9ad/actionpack/lib/action_dispatch/middleware/cookies.rb#L589-L593
• https://github.com/rails/rails/blob/1fa268bfa5667a0e9ddbfda243b5282c023ab9ad/actionpack/lib/action_dispatch/middleware/cookies.rb#L632-L636

[97jaz]

@tmilewski I'm not quite sure what you mean. If you're saying that the Cookie(Jar) code notices that the cookie is > 4k and raises an exception, then yes, it does. What I'm saying is that OmniAuth makes it easy for other parties (other than the application itself, I mean) to cause this exception by sending the user to /auth/<provider>?origin=<large string>.

This, by itself, is not a huge deal; it's just annoying. But:

Let's say you're using the ActiveRecord session store, and someone repeatedly initiates a new session and goes to /auth/<provider>?origin=<large string> and never completes authentication (so the callback phase is never run, and the session data is never removed). It seems like it would be easy to eat up a massive amount of database storage in a short time.

Also, the purpose of origin (I think, based on Saving User Location) is to be able to redirect the user to the page they had originally requested upon successful authentication. Of course, you don't want to do this blindly; if someone sends your user to /auth/<provider>?origin=<fake bank site>, you don't want to redirect the user there upon successful authentication. And it's much easier to ensure that the redirect URL is a safe one if the only party that can put the redirect URL in the session is your application (assuming that your session is tamper-proof, of course).

So OmniAuth actually makes it more difficult to (safely) save the user's location than it would be if it did not store omniauth.origin. OmniAuth basically gives you these choices:
- inspect omniauth.origin in the callback and redirect if you determine it's safe; or
- put your own return URL in the session, under a different key, and ignore omniauth.origin altogether, in which case you already know it's safe, because you chose it (again, assuming your session is tamper-proof).

But if you choose the second option and you're using a cookie store for your sessions, you need to worry about the fact that you're now trying to put two, potentially large URLs in the session, even though you only care about one of them. In which case you could add a setup phase proc that does something like this:

      setup: -> (env) {
        request = Rack::Request.new(env)
        request.update_param("origin", "X")
      }

Because if you put the empty string in origin, then OmniAuth will put the HTTP_REFERER in the session, so your best bet (as far as I can tell) is to put the shortest, non-falsey value you can there.

This isn't just a theoretical scenario. We experienced a cookie overflow for precisely this reason. At the time, we didn't know that OmniAuth was already storing the HTTP_REFERER value, and we thought, at first, that it would be a good idea to use the omniauth.origin mechanism to store the return URL. But it appears to be considerably more work to use omniauth.origin safely than it is to work around it.

That said, it's entirely possible that I'm missing something important here, and that there's a very simple way to use omniauth.origin safely.

[tmilewski]

@97jaz Let me know what you think per #912, specifically 867165a

[97jaz]

@tmilewski Yes, that's much more convenient -- thank you!

(I'd argue that the default behavior should be that there is no origin param, but I understand that that wouldn't be backwards-compatible.)

[tmilewski]

@97jaz I agree with you that the default shouldn't include origin. I'd prefer to make this backward compatible move and talk through changing defaults with everyone later.

[tmilewski]

Pushed as 1.7.0: https://rubygems.org/gems/omniauth/versions/1.7.0

Thanks!

[97jaz]

Thank you, @tmilewski!