Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Unintentional Session Hijacking in Rails 4.2.4 (Devise / Warden), Phusion Passenger 5.0.24

Background Details

We recently encountered a problem wherein User A could unintentionally hijack the session of User B who was trying to access a Controller-generated download at (nearly) the same time as User A.

We are still not 100% certain of all of the conditions necessary for this to happen, but we can reliably reproduce the problem in our production and staging environments. The important details of those environments are as follows.

Environment Details

Application Server: Phusion Passenger 5.0.21 or 5.0.24 (meaning we tried both versions and both reproduced the issue)

Framework: Rails 4.2.4

Language: Ruby 2.2.3

Operating System: CentOS 6

Interestingly, we can NOT reproduce this problem using Phusion Passenger 4.0.53.

Steps to Reproduce Hijacking

It may seem too simple for it to possibly be true, but this is all that is necessary.

  1. User A logs into the system
  2. User B logs into the system
  3. User A and B both rapidly click the same download button at (nearly) the same time

That's all it takes for someone's session to be unintentionally hijacked. (It seems like roulette as to whether A or B's session is hijacked, although it probably isn't as random as it seems.)

We know that the user's session has been hijacked because we can see the current session's user first and last name displayed on the page.

Each time, one user "becomes" the other user.

If the user access roles are different, it also means that you can now potentially have a different level of access. For instance, someone might suddenly become an admin if that's the session they unintentionally hijacked ....

Code Required

It initially seemed like Phusion Passenger was the only thing that was causing this problem because when we switched back to version 4, this issue no longer appeared.

After some code changes though, we determined that a method in our controller code seemed to contribute to this problem happening.

Here's a sample Controller method that would generate this problem on Phusion Passenger 5.0.21 or 5.0.24:

def sample_method
  respond_to do |format|
    format.csv {
      headers.merge!({'Cache-Control'=>'must-revalidate, post-check=0, pre-check=0'})
      render :text => proc { |response, output|
        100.times do |i|
          output.write("This is line #{i}\n")
        end
      }
    }
  end
end  

It seems that our modification of Cache-Control may have very well played into this issue.

Perhaps we shouldn't have modified this, but we're hoping that someone might have insights into how a cache-control parameter could have the power to suddenly plunge us into a different session.

In order to test this, you must have a route that maps to Controller#sample_method and you must have a button available to click to download this file.

I realize that we are specifying that we want a CSV and aren't returning a CSV, but I replaced our actual CSV with a proc in this case because our CSV is generated in a separate class.

The above code in the environment listed above will reproduce the issue.

Other Dependencies

We are using the Devise gem for user authentication. If you were to setup a test application to try to reproduce this problem, you'd need Devise and two accounts setup.

Incidentally, you'll also need two people on two separate computers to test this out. You'll both need to be logged into the system at the same time and try to click the button a bunch of times at the same time as well.

I realize this problem seems far-fetched, but it truly did manifest itself in our environment. It takes specific versions of Phusion Passenger, a specific set of headers, and a render block in order for it to happen, but happen it does. (The specific code is listed in the Code Required section.)

The Fix

The good news is that there is a way around this issue with code. We were able to use the #send_data method inside our format.csv block.

Instead of the other block of code, we are simply doing something along these lines:

  format.csv {
    send_data data_here, filename: filename, type: 'text/csv', disposition: 'attachment'
  }

This is cleaner code and better code. But we are still worried that there is some kind of larger issue - either in Passenger or perhaps even in our code itself.

Ideas?

Maybe an expert in the community can explain how the unintentional session hijacking like this is possible.

It seems that perhaps the session cookies are not being sent back and forth properly. (We are not using the database for our sessions.)

Although we have a fix for this particular instance of the problem, we weren't sure if there may be other underlying problems (perhaps in Passenger?) that allow this problem to manifest in the first place.

It seems like a very strange issue.

On the other hand, perhaps it's just that what we were doing with our headers was a bad idea.

Your insights are appreciated!

like image 687
aldefouw Avatar asked Feb 05 '16 21:02

aldefouw


1 Answers

Your cache-control statement allows caching (it forces revalidation, ie the browser/cache won't serve a request straight from the cache but that doesn't stop a cached response from being returned), whereas the default cache control headers rails emit contain 'private' which does not allow caching by intermediary proxies (browser caching is still allowed).

Given that the response may include the rails session cookie, caching that response and reusing it for another user results in the second user acquiring the cookie from the first user. Even if you were using a database backed session store, you'd still get the cookie identifying which row in the database to use. Any time you are displaying private content you need to be very careful with caching headers.

The reason the passenger version is relevant is that passenger 5 includes an http caching layer. Your bug is still there in passenger 4, just harder to trigger (for example 2 users behind a corporate proxy).

You should almost certainly be marking your response as private, meaning that intermediary caches (including the one in passenger) won't cache the response. Phusion wrote a blog post describing this in more detail. You can also turn off turbocaching entirely - given that by default rails marks all responses as private it may well not be doing anything useful in your app anyway.

like image 79
Frederick Cheung Avatar answered Sep 21 '22 18:09

Frederick Cheung