Background

I recently came across a pretty wild series of security holes that could have led to some crazy untraceable exploits and felt the need to document what happened. I think this is a perfect case study for how small errors and oversights can lead to much larger problems without proper review and resolution.

In some safety/security circles this is known as the Swiss Cheese Model. The analogy is pretty simple: imagine each piece of code as a “slice” of Swiss cheese. Each slice has some degree of porosity. Even for a large number of slices stacked on top of each other, sometimes the holes line up just right, and you now can see all the way to the bottom. If your stack (pun intended) is unfortunately like this, then your engineering team wins a conversation with your security team.

But wait, there’s more! If you roll the dice just right you’ll also get a data breach and all the fun that entails!

Let’s take a look at how some of these holes might line up.


First Signs of Trouble

We’re working through a refactor of our authorization library and are beginning to pick apart some things we’d like to do differently moving forward. While we’re testing some things, we notice some funny behavior in some nested routes on our user profile.

Here’s an analogous data model:

class User
  has_many :things
end

class Thing
  belongs_to :user
end

We have users who have a bunch of things. Got it. Let’s check our routes:

  resources :users do
    resources :things
  end

Normal stuff, that buys us a bunch of CRUD operations on users and things with things being nested under a user.

The authorization library being used was CanCan, and we wanted to restrict access to certain users based on role. It looked something like this:

class Ability

  include CanCan::Ability

  def initialize(admin)
    if admin.role?(:super_admin)
      can :manage, User
      can :manage, Thing
    elsif admin.role?(:admin)
      can(:manage, User) { |user| !user.restricted? }
      can :manage, Thing
    end
  end

end

:manage is a special keyword for CanCan that lets you do any named controller action on a resource. This lets super_admin administrators do anything to all users, admin administrators can do anything to any user that isn’t a restricted user, and everyone else can’t do anything.

Finally, let’s hook it all together in a controller. We’ll be looking at the ThingsController since that’s where the problems start.

class ThingsController < ApplicationController

  load_and_authorize_resource :user
  load_and_authorize_resource :thing

  # ... CRUD routes follow

end

And now we’ve run into our first problem. Because we authorize the resources in isolation (and hey, at least we authorize both of them), we never actually check if the thing we load actually belongs to the user. Normally this isn’t a problem because the Rails URL helpers are nice and we get a properly formatted URL that belongs to the correct rows in our DB:

thing = Thing.find(1)
puts thing.user_id
#=> 1

user_thing_path(thing)
#=> "/users/1/things/1"
# Basically /users/:user_id/things/:thing_id

Of course you could prepend edit_ or create_ to your URL and it’ll inject the right user ID and thing ID for you.

But what if you changed the URL manually? What if we had a couple users (one of which is restricted to super_admins), a couple things, and an admin with limited access:

user1 = User.create(restricted: true)
user2 = User.create

thing1 = user1.things.create
thing2 = user2.things.create

admin = Admin.new(roles: [:admin])

We now have an exploit on our hands. Using the routes from the path helper works just fine:

Helper Resolved Path Status Code Expected Comment
user_thing_path(thing1) /users/1/things/1 401 ✔️  
user_thing_path(thing2) /users/2/things/2 200 ✔️  
Manually altered /users/1/things/2 401 ✔️ This could have been handled more gracefully, 404 perhaps? In either case no access is granted.
Manually altered /users/2/things/1 200 Not good, we don’t check that these records are related. Because the current user is authorized to manage both individually, they are also (incorrectly) authorized in combination.

Great. What this means is that admins that have access to AT LEAST ONE user now have access to view ALL things by merely altering the user ID in the URL to one they have access to, even if that thing doesn’t belong to them.


What’s Behind Hole #2?

Let’s make matters worse, shall we?

We’ll go back in time for this one to some number of years ago when some poor developer was under massive time crunch to get things hooked up and working. In a stroke of genius our stressed engineer decides they’ll take a shortcut – they’ll reuse the form for editing a thing and just show that on both the show and edit actions. To make things “secure”, they’ll just make sure to only render the submit button if the thing is in the edit action. That’ll cut down on development time and get the feature out faster!

I hope you’re recoiling in horror at this point, but if not, let’s look at why this is bad:

If you’re using Rails you’re most likely using form_for as your form generator. This is super helpful because it automatically generates a hidden field with the CSRF token you’ll need to submit the form correctly. The CSRF token gets stored in the user session on the server and embedded in the form when rendered, and on submission, if those values match up, then the server knows the submission came from a form that it rendered. If not, then you get a nice 401 unauthorized error.

Okay, great. Let’s peek at a concrete example:

# Note that I'm removing Ruby interpolation tags here (<% %> for ERB) since the code block struggles to render them.
form_for @thing do |form|
  label_tag :field_1, "Field 1"
  if @thing.editing?
    input_tag :field_1, value: @thing.field_1
  else
    @thing.field_1
  end

  if @thing.editing?
    form.submit, "Update"
  end
end

It generates a pretty normal looking HTML form that looks like this:

<!-- /users/1/things/1/edit -->
<form class="edit_thing" id="edit_thing_1" action="/users/1/things/1" accept-charset="UTF-8" method="post">
  <input type="hidden" name="_method" value="patch">
  <input type="hidden" name="authenticity_token" value="kI/QYsCQrvpBJPjVc5Mo4y5dUhjoiWb1PoTa/egXFL8lPJ3a72upRxEohyZgZjv41le1pNj/u1krB4e6AZSw8g==">
  <label for="field_1">Field 1</label>
  <input type="text" name="field_1" id="field_1" value="FIELD 1">
  <input type="submit" name="commit" value="Update" data-disable-with="Update">
</form>

Imagine that editing? method returns false for the show view and true for the edit view. It’s actual implementation is not important.

So what does our show view look like then?

<!-- /users/1/things/1 -->
<form class="edit_thing" id="edit_thing_1" action="/users/1/things/1" accept-charset="UTF-8" method="post">
  <input type="hidden" name="_method" value="patch">
  <input type="hidden" name="authenticity_token" value="SHviKScQgZhtNFZIFdjPN54qjkDE41+I1jw+sd9AMYL9yK+RCOuGJT04KbsGLdwsZiBp/PSVgiTDv2P2NsOVzw==">
  <label for="field_1">Field 1</label>
    FIELD 1
</form>

Notice we’ve gotten rid of the two inputs (the field_1 attribute and the submit button), but all the support framework for sending a fully formed POST request to our backend is just hanging out in the open for all to see. And it’s not hard to see how to add those fields back in:

  <input type="text" name="field_1" id="field_1">
  <input type="submit" name="commit">

By adding those two lines back in via browser, we get a form that we could submit and the server would accept. Worse still, we now can submit arbitrary parameters very easily:

  <input type="text" name="field_1" id="field_1">
  <input type="text" name="field_2" id="field_2">
  <input type="submit" name="commit">

If your backend is set up to handle the field_2 param, this will update it.

Ideally, your authorization library would handle permissions to both view the edit form as well as actually process the update separately. However, remember that in our case, these two holes line up – if you have access to any user, you have access to submit this form from the show view. Our initial security hole is growing.

Because an engineer took a shortcut a long time ago and tried to DRY things up that really, really need to be separate, we now need to spend time to extract that functionality into a proper static view so that this is not exploitable going forward.

To drive this home a little further, our example admin didn’t have separate view/update permissions, but it wouldn’t be hard to imagine a case where someone who is only authorized to view a thing was able to submit a form from a page they’re already authorized to view and update data. Ideally, this form would only ever get rendered on an edit page where an additional permission check would be performed to even access it.


Digging Deeper

Let’s take another shortcut. What if we had a desire to obfuscate some sensitive data such that admins that want to view it would need to hover over the field to view the data. Once they stop hovering, the field returns to a masked state.

How should we do it?

If you answered “Put it in a password field so the browser masks it for us, then write a Javascript driver that changes the field to type="text" on hover”, then you would be correct!

I’m not going to show the JS code that handles the type swapping, as it’s pretty straightforward. I just want to point out that here, instead of dynamically swapping out the text of a span or div from a masked value to actual text, we now include a pre-generated input field for our most sensitive attributes… that has the correct name and ID (so we know what parameter the server expects)… in a valid form that can be submitted from basically anywhere… by basically anyone.

Yikes. Now you don’t even need to manipulate the DOM to add the right input, just slap in a submit button, put your value in the pre-rendered input, and go! With this pattern (along with the other holes mentioned) we’ve succeeded in making our most sensitive data the most susceptible to an exploit.

Sure, you could add the readonly attribute to the password field, but it really doesn’t help as you can just remove it and submit the form as above.


Wrap Up

I hope this has been insightful for how small issues can compound into much larger ones. Moral of the story: use the tools and patterns Rails gives you to your advantage. Rails’ motto is “convention over configuration” for a reason. If you follow the best practices from Rails, more likely than not you will have a safe and secure application. When you try to fight the Rails Way, here be dragons.

Most of these issues could be easily solved by using simple Rails generators to create the views needed instead of trying to roll a custom solution to a problem that doesn’t exist (duplicated views across show/edit aren’t actually duplicates). The permissions bug where admins had too much access to users could (and should) have been caught by automated tests.

All in all, it’s unfortunate that it happened, but it’s a great learning experience.

That’s all for now. Thanks for reading!