See all articles

Finding and Preventing Rails Authorization Bugs

This article walks through finding and fixing common Rails authorization bugs.

By OMAR

Staff Security Engineer, Betterment |

Published May. 27, 2021

Finding and Preventing Rails Authorization Bugs May 27, 2021 12:00:00 AM This article walks through finding and fixing common Rails authorization bugs. At Betterment, we build public facing applications without an authorization framework by following three principles, discussed in another blog post. Those three principles are: Authorization through Impossibility Authorization through Navigability Authorization through Application Boundaries This post will explore the first two principles and provide examples of common patterns that can lead to vulnerabilities as well as guidance for how to fix them. We will also cover the custom tools we’ve built to help avoid these patterns before they can lead to vulnerabilities. If you’d like, you can skip ahead to the tools before continuing on to the rest of this post. Authorization through Impossibility This principle might feel intuitive, but it’s worth reiterating that at Betterment we never build endpoints that allow users to access another user’s data. There is no /api/socialsecuritynumbers endpoint because it is a prime target for third-party abuse and developer error. Similarly, even our authorized endpoints never allow one user to peer into another user’s object graph. This principle keeps us from ever having the opportunity to make some of the mistakes addressed in our next section. We acknowledge that many applications out there can’t make the same design decisions about users’ data, but as a general principle we recommend reducing the ways in which that data can be accessed. If an application absolutely needs to be able to show certain data, consider structuring the endpoint in a way such that a client can’t even attempt to request another user’s data. Authorization through Navigability Rule #1: Authorization should happen in the controller and should emerge naturally from table relationships originating from the authenticated user, i.e. the “trust root chain”. This rule is applicable for all controller actions and is a critical component of our security story. If you remember nothing else, remember this. What is a “trust root chain”? It’s a term we’ve co-opted from ssl certificate lingo, and it’s meant to imply a chain of ownership from the authenticated user to a target resource. We can enforce access rules by using the affordances of our relational data without the need for any additional “permission” framework. Note that association does not imply authorization, and the onus is on the developer to ensure that associations are used properly. Consider the following controller: So long as a user is authenticated, they can perform the show action on any document (including documents belonging to others!) provided they know or can guess its ID - not great! This becomes even more dangerous if the Documents table uses sequential ids, as that would make it easy for an attacker to start combing through the entire table. This is why Betterment has a rule requiring UUIDs for all new tables. This type of bug is typically referred to as an Insecure Direct Object Reference vulnerability. In short, these bugs allow attackers to access data directly using its unique identifiers – even if that data belongs to someone else – because the application fails to take authorization into account. We can use our database relationships to ensure that users can only see their own documents. Assuming a User has many Documents then we would change our controller to the following: Now any document_id that doesn’t exist in the user’s object graph will raise a 404 and we’ve provided authorization for this endpoint without a framework - easy peezy. Rule #2: Controllers should pass ActiveRecord models, rather than ids, into the model layer. As a corollary to Rule #1, we should ensure that all authorization happens in the controller by disallowing model initialization with *_id attributes. This rule speaks to the broader goal of authorization being obvious in our code. We want to minimize the hops and jumps required to figure out what we’re granting access to, so we make sure that it all happens in the controller. Consider a controller that links attachments to a given document. Let’s assume that a User has many Attachments that can be attached to a Document they own. Take a minute and review this controller - what jumps out to you? At first glance, it looks like the developer has taken the right steps to adhere to Rule #1 via the document method and we’re using strong params, is that enough? Unfortunately, it’s not. There’s actually a critical security bug here that allows the client to specify any attachment_id, even if they don’t own that attachment - eek! Here’s simple way to resolve our bug: Now before we create a new AttachmentLink, we verify that the attachment_id specified actually belongs to the user and our code will raise a 404 otherwise - perfect! By keeping the authorization up front in the controller and out of the model, we’ve made it easier to reason about. If we buried the authorization within the model, it would be difficult to ensure that the trust-root chain is being enforced – especially if the model is used by multiple controllers that handle authorization inconsistently. Reading the AttachmentLink model code, it would be clear that it takes an attachment_id but whether authorization has been handled or not would remain a bit of a mystery. Automatically Detecting Vulnerabilities At Betterment, we strive to make it easy for engineers to do the right thing – especially when it comes to security practices. Given the formulaic patterns of these bugs, we decided static analysis would be a worthwhile endeavor. Static analysis can help not only with finding existing instances of these vulnerabilities, but also prevent new ones from being introduced. By automating detection of these “low hanging fruit” vulnerabilities, we can free up engineering effort during security reviews and focus on more interesting and complex issues. We decided to lean on RuboCop for this work. As a Rails shop, we already make heavy use of RuboCop. We like it because it’s easy to introduce to a codebase, violations break builds in clear and actionable ways, and disabling specific checks requires engineers to comment their code in a way that makes it easy to surface during code review. Keeping rules #1 and #2 in mind, we’ve created two cops: Betterment/UnscopedFind and Betterment/AuthorizationInController; these will flag any models being retrieved and created in potentially unsafe ways, respectively. At a high level, these cops track user input (via params.permit et al.) and raise offenses if any of these values get passed into methods that could lead to a vulnerability (e.g. model initialization, find calls, etc). You can find these cops here. We’ve been using these cops for over a year now and have had a lot of success with them. In addition to these two, the Betterlint repository contains other custom cops we’ve written to enforce certain patterns -- both security related as well as more general ones. We use these cops in conjunction with the default RuboCop configurations for all of our Ruby projects. Let’s run the first cop, Betterment/UnscopedFind against DocumentsController from above: $ rubocop app/controllers/documents_controller.rb Inspecting 1 file C Offenses: app/controllers/documents_controller.rb:3:17: C: Betterment/UnscopedFind: Records are being retrieved directly using user input. Please query for the associated record in a way that enforces authorization (e.g. "trust-root chaining"). INSTEAD OF THIS: Post.find(params[:post_id]) DO THIS: currentuser.posts.find(params[:postid]) See here for more information on this error: https://github.com/Betterment/betterlint/blob/main/README.md#bettermentunscopedfind @document = Document.find(params[:document_id]) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1 file inspected, 1 offense detected The cop successfully located the vulnerability. If we attempted to deploy this code, RuboCop would fail the build, preventing the code from going out while letting reviewers know exactly why. Now let’s try running Betterment/AuthorizationInController on the AttachmentLink example from earlier: $ rubocop app/controllers/documents/attachments_controller.rb Inspecting 1 file C Offenses: app/controllers/documents/attachments_controller.rb:3:24: C: Betterment/AuthorizationInController: Model created/updated using unsafe parameters. Please query for the associated record in a way that enforces authorization (e.g. "trust-root chaining"), and then pass the resulting object into your model instead of the unsafe parameter. INSTEAD OF THIS: postparameters = params.permit(:albumid, :caption) Post.new(post_parameters) DO THIS: album = currentuser.albums.find(params[:albumid]) post_parameters = params.permit(:caption).merge(album: album) Post.new(post_parameters) See here for more information on this error: https://github.com/Betterment/betterlint/blob/main/README.md#bettermentauthorizationincontroller AttachmentLink.new(create_params.merge(document: document)).save! ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1 file inspected, 1 offense detected The model initialization was flagged because it was seen using create_params, which contains user input. Like with the other cop, this would fail the build and prevent the code from making it to production. You may have noticed that unlike the previous example, the vulnerable code doesn’t directly reference a params.permit call or any of the parameter names, but the code was still flagged. This is because both of the cops keep a little bit of state to ensure they have the appropriate context necessary when analyzing potentially unsafe function calls. We also made sure that when developing these cops that we tested them with real code samples and not just contrived scenarios that no developer would actually ever attempt. False Positives With any type of static analysis, there’s bound to be false positives. When working on these cops, we narrowed down false positives to two scenarios: The flagged code could be considered insecure only in other contexts: e.g. the application or models in question don’t have a concept of “private” data The flagged code isn’t actually insecure: e.g. the initialization happens to take a parameter whose name ends in _id but it doesn’t refer to a unique identifier for any objects In both these cases, the developer should feel empowered to either rewrite the line in question or locally disable the cop, both of which will prevent the code from being flagged. Normally we’d consider opting out of security analysis to be an unsafe thing to do, but we actually like the way RuboCop handles this because it can help reduce some code review effort; the first solution eliminates the vulnerable-looking pattern (even if it wasn’t a vulnerability to begin with) while the second one signals to reviewers that they should confirm this code is actually safe (making it easy to pinpoint areas of focus). Testing & Code Review Strategies Rubocop and Rails tooling can only get us so far in mitigating authorization bugs. The remainder falls on the shoulders of the developer and their peers to be cognizant of the choices they are making when shipping new application controllers. In light of that, we’ll cover some helpful strategies for keeping authorization front of mind. Testing When writing request specs for a controller action, write a negative test case to prove that attempts to circumvent your authorization measures return a 404. For example, consider a request spec for our Documents::AttachmentsController: These test cases are an inexpensive way to prove to yourself and your reviewers that you’ve considered the authorization context of your controller action and accounted for it properly. Like all of our tests, this functions both as regression prevention and as documentation of your intent. Code Review Our last line of defense is code review. Security is the responsibility of every engineer, and it’s critical that our reviewers keep authorization and security in mind when reviewing code. A few simple questions can facilitate effective security review of a PR that touches a controller action: Who is the authenticated user? What resource is the authenticated user operating on? Is the authenticated user authorized to operate on the resource in accordance with Rule #1? What parameters is the authenticated user submitting? Where are we authorizing the user’s access to those parameters? Do all associations navigated in the controller properly signify authorization? Getting in the habit of asking these questions during code review should lead to more frequent conversations about security and data access. Our hope is that linking out to this post and its associated Rules will reinforce a strong security posture in our application development. In Summary Unlike authentication, authorization is context specific and difficult to “abstract away” from the leaf nodes of application code. This means that application developers need to consider authorization with every controller we write or change. We’ve explored two new rules to encourage best practices when it comes to authorization in our application controllers: Authorization should happen in the controller and should emerge naturally from table relationships originating from the authenticated user, i.e. the “trust root chain”. Controllers should pass ActiveRecord models, rather than ids, into the model layer. We’ve also covered how our custom cops can help developers avoid antipatterns, resulting in safer and easier to read code. Keep these in mind when writing or reviewing application code that an authenticated user will utilize and remember that authorization should be clear and obvious.

At Betterment, we build public facing applications without an authorization framework by following three principles, discussed in another blog post. Those three principles are:

Authorization through Impossibility
Authorization through Navigability
Authorization through Application Boundaries

This post will explore the first two principles and provide examples of common patterns that can lead to vulnerabilities as well as guidance for how to fix them. We will also cover the custom tools we’ve built to help avoid these patterns before they can lead to vulnerabilities. If you’d like, you can skip ahead to the tools before continuing on to the rest of this post.

Authorization through Impossibility

This principle might feel intuitive, but it’s worth reiterating that at Betterment we never build endpoints that allow users to access another user’s data. There is no /api/socialsecuritynumbers endpoint because it is a prime target for third-party abuse and developer error. Similarly, even our authorized endpoints never allow one user to peer into another user’s object graph. This principle keeps us from ever having the opportunity to make some of the mistakes addressed in our next section.

We acknowledge that many applications out there can’t make the same design decisions about users’ data, but as a general principle we recommend reducing the ways in which that data can be accessed. If an application absolutely needs to be able to show certain data, consider structuring the endpoint in a way such that a client can’t even attempt to request another user’s data.

Authorization through Navigability

Rule #1: Authorization should happen in the controller and should emerge naturally from table relationships originating from the authenticated user, i.e. the “trust root chain”.

This rule is applicable for all controller actions and is a critical component of our security story. If you remember nothing else, remember this.

What is a “trust root chain”? It’s a term we’ve co-opted from ssl certificate lingo, and it’s meant to imply a chain of ownership from the authenticated user to a target resource. We can enforce access rules by using the affordances of our relational data without the need for any additional “permission” framework. Note that association does not imply authorization, and the onus is on the developer to ensure that associations are used properly.

Consider the following controller:

So long as a user is authenticated, they can perform the show action on any document (including documents belonging to others!) provided they know or can guess its ID - not great! This becomes even more dangerous if the Documents table uses sequential ids, as that would make it easy for an attacker to start combing through the entire table. This is why Betterment has a rule requiring UUIDs for all new tables.

This type of bug is typically referred to as an Insecure Direct Object Reference vulnerability. In short, these bugs allow attackers to access data directly using its unique identifiers – even if that data belongs to someone else – because the application fails to take authorization into account.

We can use our database relationships to ensure that users can only see their own documents. Assuming a User has many Documents then we would change our controller to the following:

Now any document_id that doesn’t exist in the user’s object graph will raise a 404 and we’ve provided authorization for this endpoint without a framework - easy peezy.

Rule #2: Controllers should pass ActiveRecord models, rather than ids, into the model layer.

As a corollary to Rule #1, we should ensure that all authorization happens in the controller by disallowing model initialization with *_id attributes.

This rule speaks to the broader goal of authorization being obvious in our code. We want to minimize the hops and jumps required to figure out what we’re granting access to, so we make sure that it all happens in the controller.

Consider a controller that links attachments to a given document. Let’s assume that a User has many Attachments that can be attached to a Document they own.

Take a minute and review this controller - what jumps out to you?

At first glance, it looks like the developer has taken the right steps to adhere to Rule #1 via the document method and we’re using strong params, is that enough?

Unfortunately, it’s not. There’s actually a critical security bug here that allows the client to specify any attachment_id, even if they don’t own that attachment - eek! Here’s simple way to resolve our bug:

Now before we create a new AttachmentLink, we verify that the attachment_id specified actually belongs to the user and our code will raise a 404 otherwise - perfect! By keeping the authorization up front in the controller and out of the model, we’ve made it easier to reason about.

If we buried the authorization within the model, it would be difficult to ensure that the trust-root chain is being enforced – especially if the model is used by multiple controllers that handle authorization inconsistently. Reading the AttachmentLink model code, it would be clear that it takes an attachment_id but whether authorization has been handled or not would remain a bit of a mystery.

Automatically Detecting Vulnerabilities

At Betterment, we strive to make it easy for engineers to do the right thing – especially when it comes to security practices. Given the formulaic patterns of these bugs, we decided static analysis would be a worthwhile endeavor. Static analysis can help not only with finding existing instances of these vulnerabilities, but also prevent new ones from being introduced. By automating detection of these “low hanging fruit” vulnerabilities, we can free up engineering effort during security reviews and focus on more interesting and complex issues.

We decided to lean on RuboCop for this work. As a Rails shop, we already make heavy use of RuboCop. We like it because it’s easy to introduce to a codebase, violations break builds in clear and actionable ways, and disabling specific checks requires engineers to comment their code in a way that makes it easy to surface during code review.

Keeping rules #1 and #2 in mind, we’ve created two cops: Betterment/UnscopedFind and Betterment/AuthorizationInController; these will flag any models being retrieved and created in potentially unsafe ways, respectively. At a high level, these cops track user input (via params.permit et al.) and raise offenses if any of these values get passed into methods that could lead to a vulnerability (e.g. model initialization, find calls, etc).

You can find these cops here. We’ve been using these cops for over a year now and have had a lot of success with them. In addition to these two, the Betterlint repository contains other custom cops we’ve written to enforce certain patterns -- both security related as well as more general ones. We use these cops in conjunction with the default RuboCop configurations for all of our Ruby projects.

Let’s run the first cop, Betterment/UnscopedFind against DocumentsController from above:

$ rubocop app/controllers/documents_controller.rb

Inspecting 1 file

Offenses:

app/controllers/documents_controller.rb:3:17: C: Betterment/UnscopedFind: Records are being retrieved directly using user input.

Please query for the associated record in a way that enforces authorization (e.g. "trust-root chaining").

INSTEAD OF THIS:

Post.find(params[:post_id])

DO THIS:

currentuser.posts.find(params[:postid])

See here for more information on this error:

https://github.com/Betterment/betterlint/blob/main/README.md#bettermentunscopedfind

@document = Document.find(params[:document_id])

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

The cop successfully located the vulnerability. If we attempted to deploy this code, RuboCop would fail the build, preventing the code from going out while letting reviewers know exactly why.

Now let’s try running Betterment/AuthorizationInController on the AttachmentLink example from earlier:

$ rubocop app/controllers/documents/attachments_controller.rb

Inspecting 1 file

Offenses:

app/controllers/documents/attachments_controller.rb:3:24: C: Betterment/AuthorizationInController: Model created/updated using unsafe parameters.

Please query for the associated record in a way that enforces authorization (e.g. "trust-root chaining"),

and then pass the resulting object into your model instead of the unsafe parameter.

INSTEAD OF THIS:

postparameters = params.permit(:albumid, :caption)

Post.new(post_parameters)

DO THIS:

album = currentuser.albums.find(params[:albumid])

post_parameters = params.permit(:caption).merge(album: album)

Post.new(post_parameters)

See here for more information on this error:

https://github.com/Betterment/betterlint/blob/main/README.md#bettermentauthorizationincontroller

AttachmentLink.new(create_params.merge(document: document)).save!

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

The model initialization was flagged because it was seen using create_params, which contains user input. Like with the other cop, this would fail the build and prevent the code from making it to production.

You may have noticed that unlike the previous example, the vulnerable code doesn’t directly reference a params.permit call or any of the parameter names, but the code was still flagged. This is because both of the cops keep a little bit of state to ensure they have the appropriate context necessary when analyzing potentially unsafe function calls. We also made sure that when developing these cops that we tested them with real code samples and not just contrived scenarios that no developer would actually ever attempt.

False Positives

With any type of static analysis, there’s bound to be false positives. When working on these cops, we narrowed down false positives to two scenarios:

The flagged code could be considered insecure only in other contexts: e.g. the application or models in question don’t have a concept of “private” data
The flagged code isn’t actually insecure: e.g. the initialization happens to take a parameter whose name ends in _id but it doesn’t refer to a unique identifier for any objects

In both these cases, the developer should feel empowered to either rewrite the line in question or locally disable the cop, both of which will prevent the code from being flagged. Normally we’d consider opting out of security analysis to be an unsafe thing to do, but we actually like the way RuboCop handles this because it can help reduce some code review effort; the first solution eliminates the vulnerable-looking pattern (even if it wasn’t a vulnerability to begin with) while the second one signals to reviewers that they should confirm this code is actually safe (making it easy to pinpoint areas of focus).

Testing & Code Review Strategies

Rubocop and Rails tooling can only get us so far in mitigating authorization bugs. The remainder falls on the shoulders of the developer and their peers to be cognizant of the choices they are making when shipping new application controllers. In light of that, we’ll cover some helpful strategies for keeping authorization front of mind.

Testing

When writing request specs for a controller action, write a negative test case to prove that attempts to circumvent your authorization measures return a 404. For example, consider a request spec for our Documents::AttachmentsController:

controller request spec

These test cases are an inexpensive way to prove to yourself and your reviewers that you’ve considered the authorization context of your controller action and accounted for it properly. Like all of our tests, this functions both as regression prevention and as documentation of your intent.

Code Review

Our last line of defense is code review. Security is the responsibility of every engineer, and it’s critical that our reviewers keep authorization and security in mind when reviewing code. A few simple questions can facilitate effective security review of a PR that touches a controller action:

Who is the authenticated user?
What resource is the authenticated user operating on?
Is the authenticated user authorized to operate on the resource in accordance with Rule #1?
What parameters is the authenticated user submitting?
Where are we authorizing the user’s access to those parameters?
Do all associations navigated in the controller properly signify authorization?

Getting in the habit of asking these questions during code review should lead to more frequent conversations about security and data access. Our hope is that linking out to this post and its associated Rules will reinforce a strong security posture in our application development.

In Summary

Unlike authentication, authorization is context specific and difficult to “abstract away” from the leaf nodes of application code. This means that application developers need to consider authorization with every controller we write or change. We’ve explored two new rules to encourage best practices when it comes to authorization in our application controllers:

Authorization should happen in the controller and should emerge naturally from table relationships originating from the authenticated user, i.e. the “trust root chain”.
Controllers should pass ActiveRecord models, rather than ids, into the model layer.

We’ve also covered how our custom cops can help developers avoid antipatterns, resulting in safer and easier to read code.

Keep these in mind when writing or reviewing application code that an authenticated user will utilize and remember that authorization should be clear and obvious.