Missing owner checks and an empty workspace ID

Most of what I find on this program starts with one question: when I click something, which request fires, and which ID is in it? The program is private. Its main product is a large collaboration platform built around workspaces, projects, files and comments, and asking that question across it gave me the four bugs below.

They are all the same class. The server authenticated the request, then acted on whatever object the client named without checking that the object belonged to the caller. OWASP’s API list calls it broken object level authorization. Most hunters still say IDOR.

Editing any user’s profile

The account page saves name, bio, company, job title, address, website, language and email preferences through one mutation, and every save carries the user’s ID in the body:

{"input": {"id": "<user-id>", "name": "New Name"}}

The ID is long and random, so guessing it was never going to work, and I almost moved on. What kept me there was asking where else the ID shows up. I opened another user’s public community profile with the proxy running and the same ID was sitting in the response.

From there it was two steps. Take the victim’s ID from their public profile, put it in my own save request, send it. The server renamed them. It checked that I had a valid session and never compared the ID in the body with the user that session belonged to.

Random IDs make the first step harder but don’t touch the second. IDs leak through public profiles, list responses, share links and error messages, and once one leaks it doesn’t matter how random it is.

I made a mistake with this one. I reported every field separately, nine reports for a single missing check. It should have been one report saying any user’s profile can be modified, with the fields listed inside.

Sequential plan IDs

A smaller planning tool in the same program numbered its plans sequentially. I created a plan, deleted it from the UI, caught the request and subtracted one from the ID:

{"operationName": "DeletePlan", "variables": {"id": "10432"}}

Someone else’s plan was gone.

The more useful part came after that. Every control on a plan card is its own mutation: edit, rename, archive, unarchive, share, edit notes, cancel a transfer. Each one takes the plan ID, and none of them checked who owned the plan. The transfer mutation also returned the email address the plan was being transferred to, so it leaked data on top of everything else.

GraphQL APIs often do authorization inside each resolver, so every new mutation needs its own check, and it’s easy for one to ship without it. When one action on an object is missing the check, I go through every other action on that object before writing anything up.

Deleting other people’s comments

Files can have comments. The comment list returns every comment’s ID, and the delete call puts that ID in the path:

DELETE /api/v1/files/<file-id>/comments/<victim-comment-id> HTTP/1.1
Authorization: Bearer <attacker-token>

As the attacker in a shared project I read the comment IDs from the list response, deleted one of my own comments to capture the request, then sent it again with the victim’s comment ID. Their comment was deleted.

A list endpoint that returns IDs, sitting next to an action endpoint that accepts them, is the most reliable IDOR pattern I know of. The list gives you every ID you need.

An empty scope ID

This is my favorite of the four. A GraphQL mutation creates a new version of a file, with a description. Besides the target file it takes a workspaceId, and the server uses that to decide whether I’m allowed to write to the file. With my own workspace ID and someone else’s file it refused, which is correct.

Then I kept the key and removed the value:

{"workspaceId": "", "targetId": "<victim-file-id>", "description": "changed by me"}

The new version was created on the victim’s file.

I can’t see their code, but the behaviour fits a check that only runs when a scope is supplied. In JavaScript an empty string is falsy, so a guard like the one below skips the whole check:

async function createVersion(input, user) {
  if (input.workspaceId) {
    await assertCanWrite(user, input.workspaceId, input.targetId);
  }
  return versions.create(input.targetId, input.description);
}

Really the server shouldn’t take the scope from the client in the first place. It already knows which workspace the file belongs to. If it loads the file, reads the workspace from the database and checks membership against that, there is no field left to empty.

Since this one, whenever a check blocks me I try it with nothing: an empty string, null, the key removed, an empty array, an array where a string is expected. Validation tends to be written for the values the client normally sends.

Fix

Every handler that takes an object ID has to load that object and check it against the caller before doing anything with it. The simplest way to make that hard to forget is to put the owner in the query:

const plan = await db.plans.findOne({ id: input.id, owner: user.id });
if (!plan) throw new NotFound(); // same answer for "missing" and "not yours"

Giving the same answer for “does not exist” and “not yours” also stops the endpoint from confirming which IDs are real.

A missing or empty scope value should fail validation. It should never mean the check is skipped.

Disclosure

All of this was reported privately to the vendor. The vendor isn’t named here, and paths, field names and IDs are changed. Each request has the same structure as the original.