The Most Overlooked Server Permission Checks

The Most Overlooked Server Permission Checks

After reviewing the code for hundreds of backend server applications, we’ve seen some recurring permissions issues. Below we’ll break down what are the most common problems and how to address them.

We previously looked at common server authentication issues we see in code review and offered tips to avoid them. If you followed these suggestions, you should have improved your server authentication techniques and can assure all your incoming requests have a validated user attached.

Next, we’ll discuss some common pitfalls for extending trust to that user so they can only access resources they are supposed to. We’ll use a similar example to the last article. Imagine we are using a hypothetical Python/Django app, but this could apply to almost any language and web framework.

For this scenario, let’s say we want to build an API that enables a user to get the contents of one of their private repositories. Below is an example of how someone might go about building this API.

Let’s say they set the route for the API to have a pattern like this (where user_id​ and repo_name​ are parameters passed in the request):

GET  /user/{user_id}/repos/{repo_name}/contents

Remember to check for permissions

Let’s write a naive implementation of this API we described above: ​​​​

@login_required
​​def get_repo_contents(request, user_id, repo_name):
​​    repo = Repo.objects.get(name=repo_name)
​​    return repo.content

​​In the above example, there are no permissions checks in place. The requesting user could be any logged user as we are just blindly looking up the repository and returning contents. Skipping the permission check allows any user in your system to now see the contents of any other user’s private repository. ​​​​

This time let’s add a permission check to the request:

​​​​@login_required
​​def get_repo_contents(request, user_id, repo_name):
​​    user = User.objects.get(id=user_id)
​​    repo = user.repo_set.get(name=repo_name)
​​    return repo.contents

​​​At first glance, this example looks like this would properly check a user’s permissions.

The API requires authentication, and we take the user_id​ passed and then filter the repo​ objects to only include those that are related/owned by the user before we look up the repo_name​.

This filtering means that for this user we are looking up we will only acknowledge the existence of a repo if it is in their set of repos they have permissions to access. Otherwise, it will throw a 404-type error.

​​While this example above looks like it now secures access, there is a widespread problem. We are not extending the trust of our authentication that we worked so hard to get correct. We are authenticating “a user,” yet the route URL includes another user parameter in it that we are using for the permissions check.

This lack of further authentication means that an attacker could see the contents of other user’s private repositories by making a single user account, guess user_ids of other users, and call this API.​​

Extending your trust

One of the best approaches for permissions I describe as “extending your trust.” This means to start from a place of trust, which is your authenticated user for the request. Then build out queries/relations from the user to whatever object you are trying to access. Each step of the way you only grant access to things the requesting user has access to or “owns.”

The fact we included the user_id​ in the API URL should be a quick code-smell for issues like this. Unless you are building a publicly shareable system, you should not usually need to include this in the API URL.

Because of this, we should update the API URL to look like this:

GET  /repos/{repo_name}/contents
@login_required
​​def get_repo_contents(request, repo_name):
​​    # using the request.user now, that is the one that our authentication has verified
​​    user = request.user
​​    repo = user.repo_set.get(name=repo_name)
​​    return repo.contents

​​​As you can see above, we are taking the request.user​ and extending from there to get our repos that we have permission to access. ​​

Additional layers of defense

​​These examples show how easy it is to write code that looks like it is doing all the steps of authorization when it has several possible access issues. To further protect your resources you should strive to add multiple layers of defense on top of the suggestions above.

One way is to use a secure id, such as a randomly generated UUID string (version 4 UUID) for identifying users. If someone accidentally introduces a permissions error and was using this type of UUIDs instead of ids, the surface area of exposure may not be as bad.

For example, with auto-incrementing user_ids, an attacker could try user_ids starting at 1, incrementally increase to 2, then 3, and so on until they found a user_id with permission. On the other hand, if you use UUIDs, they would have to know the exact UUID for the user beforehand, as guessing a random string is near impossible. However, this should just be used as an extra layer of defense and is not secure enough on its own to define permissions as it is possible that you could leak the UUIDs to other users in some other APIs.

Also, with normal unit test coverage, permissions and authorization issues are challenging to catch. It’s likely that if you have unit tests, you are using fake ids that would pass even with the issues above. So it’s a good idea to add tests representing cases where an attacker is trying to break your permissions as shown in the examples above.

Even if you miss some of the above protections, code review is one of the best places to catch these kinds of issues. A fresh set of eyes can read over your code and is more likely to find things that seem out of place and lead to these types of vulnerabilities.


About PullRequest

HackerOne PullRequest is a platform for code review, built for teams of all sizes. We have a network of expert engineers enhanced by AI, to help you ship secure code, faster.

Learn more about PullRequest

Tyler Mann headshot
by Tyler Mann

August 17, 2018