Other customers' invoices through the API: a missing ownership check

The portal is used by the wholesaler's business customers. Each customer company has one or more users. Signing in creates a server-side session holding the user ID and the ID of their company. The browser holds only a random session identifier in a cookie.

Invoices live in an invoices table with a company_id column. Their identifiers are sequential integers. The application logs the user ID, method and path of every request.

Before adding payments to the portal, the company commissioned a penetration test. The tester was given two test accounts in two different customer companies. Signed in with the first, changing the number in GET /api/invoices/1841 to an invoice of the second company returned it in full: the buyer, the address, the line items and the amounts. The PDF download and the delivery note detail behaved the same way.

Prerequisite: the attacker needs any valid portal account, so they must be a customer or have taken over a customer's account. Because the numbers are sequential, other invoices could simply be walked in order. Changing or deleting them was not possible; the portal does not offer customers those operations at all.

Impact: disclosure of business and personal data of every portal customer. Had the flaw been exploited, it would be a personal data breach to be assessed under the GDPR. OWASP tracks this class as Broken Object Level Authorization (API1:2023).

Authentication, meaning verifying who is signed in, was sound. Authorisation, meaning checking whether this user may access this particular record, was missing. The requireAuth middleware only checked that a session existed.

The query selected an invoice by the ID from the URL alone: SELECT ... FROM invoices WHERE id = $1. It was parameterised, so there was no SQL injection, but nothing restricted the result to the signed-in user's company. The invoice list did filter on company_id. The detail, the PDF and the delivery note were added later, each a little differently, and the condition was forgotten.

First, every endpoint that loads a record by an ID from the request was reviewed for whether its query also includes the company from the session. Exactly three were affected: the invoice detail, the PDF and the delivery note detail.

Next, the question of whether anyone had used it. The last 30 days of logs were joined with the database to find requests where the invoice in the path belonged to a company other than the user's. Apart from the test accounts, there were none.

For anything older than 30 days this cannot be checked, because the logs no longer exist. The incident record says exactly that. It does not claim the flaw was never exploited; it claims only what can be shown.

Every query for a record that belongs to a company now carries a company condition. The company ID comes only from the server-side session, never from the request body or parameters. Access to these tables goes through a small layer of functions that take the company ID as a required argument.

// data layer: the company is a required argument, not an afterthought
export async function getInvoice(companyId, invoiceId) {
  const { rows } = await pool.query(
    `SELECT id, number, issued_at, total
       FROM invoices
      WHERE id = $1 AND company_id = $2`,
    [invoiceId, companyId],
  );
  return rows[0] ?? null;
}

// route (Express 5 passes a rejected promise on to the error handler)
app.get("/api/invoices/:id", requireAuth, async (req, res) => {
  if (!/^\d+$/.test(req.params.id)) return res.sendStatus(404);

  // companyId comes from the server-side session, never from the request
  const invoice = await getInvoice(req.session.companyId, req.params.id);

  // "does not exist" and "not yours" must look the same from outside
  if (!invoice) return res.sendStatus(404);
  res.json(invoice);
});

A missing invoice and somebody else's invoice both return 404. If the latter returned 403, it could be used to learn which invoice numbers exist.

So that the flaw does not return with the next endpoint, integration tests with two companies were added. They run in CI on every pull request, and the team's rule is not to merge anything that breaks them.

import { test } from "node:test";
import request from "supertest";
import { app } from "../src/app.js";
import { seedTwoCompanies } from "./fixtures.js";

test("a user of company A cannot read company B's invoice", async () => {
  const { userA, invoiceB } = await seedTwoCompanies();
  const agent = request.agent(app); // keeps the session cookie

  await agent.post("/api/login").send(userA.credentials).expect(200);
  await agent.get(`/api/invoices/${invoiceB.id}`).expect(404);
  await agent.get(`/api/invoices/${invoiceB.id}/pdf`).expect(404);
});

Alternatives considered: PostgreSQL Row Level Security would add the check in the database itself, but it needs the company context set on every transaction, and the application must not connect as the table owner, to whom policies do not apply by default. For two developers an explicit filter with tests was easier to read. Random UUIDs instead of sequential numbers would make guessing harder, but they do not replace authorisation.

The database returns a row only when both the invoice ID and the company match. The company is set by the server-side session, whose contents the client cannot change. Changing the number in the URL therefore only ever yields 404.

The required companyId argument moves the check out of the developer's memory and into the code: the function cannot be called without a company. The two-company tests catch the case where someone bypasses the layer and writes a query directly.

What it does not address: whoever takes over a customer's account, for instance with a guessed password, will see that company's invoices, because that account genuinely has access to them.