Software Securityhome
Fix a findingSearch
Fix card · A01:2025 Broken Access Control

Fix missing object-level authorization (IDOR) in Node.js and Express

The vulnerable code, the fix, a plausible fix that still fails, and the regression tests that tell them apart.

In short

Take the user from your authentication middleware (req.user), never from the request, and put the same scope in every invoice query: tenant_id = $tenant AND (owner_id = $user OR $isBillingAdmin). Detail, list and export then return only rows the caller may read. Answer 404 for anything else, and refuse an export when any requested ID is out of scope.

Stack
  • Language JavaScript (ES modules)
  • Runtime Node.js 26.11.1 (node:sqlite needs 22.13.0 or later without a flag)
  • Framework Express 5.3.0
Maps to
Verified
Tests pass · fixed, vulnerable and wrong runs as expected
Other stacks9 cards
Download the project ZIP, 25 KB
On this page

Steps

  1. List every route that reads an invoice: detail, list, search, export, download, and any job that runs on a user's behalf. Note where each one gets the invoice IDs and the user.
  2. Run authentication before the routes (app.use(authenticate)) so every handler has req.user = { id, tenantId, role } built from server-side data. Delete any code that takes a user ID, tenant ID or role from req.body, req.query or a header.
  3. Make every data-access function that touches invoices take the user as a parameter and add tenant_id = $tenant AND (owner_id = $user OR $isBillingAdmin) to its WHERE clause, with the values bound from req.user.
  4. Return 404 when the scoped query finds nothing, so a missing invoice and someone else's look the same.
  5. For bulk endpoints, compare the rows the scoped query returned with the IDs that were asked for, and refuse the whole request if any are missing.
  6. Add regression tests that sign in as a colleague, as a member of another tenant and as a billing admin of another tenant, and replay invoice IDs against the detail, list and export routes.

Vulnerable, fixed and wrong

The three implementations share one API and one test suite; only the code below changes. The tests load the one that IMPL names.

The vulnerable code

Vulnerableimpl/vulnerable.jsJavaScript
export function getInvoice(db, user, id) {
  return db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    WHERE id = ?`)
    .get(id);
}

export function listInvoices(db, user) {
  return db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    WHERE tenant_id = ?
    ORDER BY id`)
    .all(user.tenantId);
}

export function exportInvoices(db, user, ids) {
  return db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    WHERE id IN (SELECT value FROM json_each(?))
    ORDER BY id`)
    .all(JSON.stringify(ids));
}

With IMPL=vulnerable, 6 of 15 tests fail (6 exploit tests).

The fix

Fixedimpl/fixed.jsJavaScript
// Members read their own invoices; billing admins read every invoice in their own
// tenant. The tenant test sits outside the parentheses, so it binds admins too:
//   tenant_id = $tenant AND (owner_id = $user OR $isBillingAdmin)
function scope(user) {
  // node:sqlite binds booleans only from Node.js 26.8.0, so pass 1 or 0.
  return { tenant: user.tenantId, user: user.id, isBillingAdmin: user.role === 'billing_admin' ? 1 : 0 };
}

export function getInvoice(db, user, id) {
  return db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    WHERE id = $id AND tenant_id = $tenant AND (owner_id = $user OR $isBillingAdmin)`)
    .get({ id, ...scope(user) });
}

export function listInvoices(db, user) {
  return db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    WHERE tenant_id = $tenant AND (owner_id = $user OR $isBillingAdmin)
    ORDER BY id`)
    .all(scope(user));
}

export function exportInvoices(db, user, ids) {
  return db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    WHERE id IN (SELECT value FROM json_each($ids))
      AND tenant_id = $tenant AND (owner_id = $user OR $isBillingAdmin)
    ORDER BY id`)
    .all({ ids: JSON.stringify(ids), ...scope(user) });
}

With IMPL=fixed, 15 of 15 tests pass.

A fix that looks safe but is not

Wrong fiximpl/wrong.jsJavaScript
function canRead(user, invoice) {
  return invoice.owner_id === user.id || user.role === 'billing_admin';
}

export function getInvoice(db, user, id) {
  const invoice = db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    WHERE id = ?`)
    .get(id);
  return invoice && canRead(user, invoice) ? invoice : undefined;
}

export function listInvoices(db, user) {
  return db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    ORDER BY id`)
    .all()
    .filter((invoice) => canRead(user, invoice));
}

export function exportInvoices(db, user, ids) {
  return db.prepare(`
    SELECT id, tenant_id, owner_id, number, amount_cents, status FROM invoices
    WHERE id IN (SELECT value FROM json_each(?))
    ORDER BY id`)
    .all(JSON.stringify(ids))
    .filter((invoice) => canRead(user, invoice));
}

With IMPL=wrong, 3 of 15 tests fail (3 exploit tests).

Routes: the user comes from authentication, not the request

app.jsJavaScript
// authenticate is your session or token middleware. It sets req.user to
// { id, tenantId, role } from server-side data or answers 401. The routes hand
// req.user to the invoice module and never read a user, tenant or role from the request.
app.use(authenticate);

app.get('/invoices', (req, res) => {
  res.json(impl.listInvoices(db, req.user));
});

app.get('/invoices/export', (req, res) => {
  const ids = parseIds(req.query.ids);
  if (ids === undefined) {
    return res.status(400).json({ error: `ids must be 1 to ${MAX_EXPORT_IDS} invoice IDs, comma-separated` });
  }
  const invoices = impl.exportInvoices(db, req.user, ids);
  // All or nothing: one missing or unreadable ID refuses the whole export, so a
  // partial file is never mistaken for the full one.
  if (invoices.length !== ids.length) return notFound(res);
  res.type('text/csv').send(toCsv(invoices));
});

app.get('/invoices/:id', (req, res) => {
  const id = /^\d{1,9}$/.test(req.params.id) ? Number(req.params.id) : undefined;
  const invoice = id === undefined ? undefined : impl.getInvoice(db, req.user, id);
  // Missing and not yours get the same answer, so IDs cannot be probed.
  if (invoice === undefined) return notFound(res);
  res.json(invoice);
});

The regression test

The exploit tests send the attack and check that it fails; functional tests next to them keep ordinary requests working, so a fix that rejects everything fails too.

Testtest/invoices.test.jsJavaScript
describe(`exploit (IMPL=${implName})`, () => {
  // Denied looks exactly like missing: 404 and none of the invoice's data.
  async function assertHidden(user, path, number) {
    const res = await get(user, path);
    assert.equal(res.status, 404, `expected 404, got ${res.status} ${res.body}`);
    assert.ok(!res.body.includes(number), `response leaked ${number}`);
  }

  test("a member cannot read a colleague's invoice in the same tenant", async () => {
    await assertHidden('alice', '/invoices/502', 'INV-502');
  });

  test('a member of another tenant cannot read the invoice', async () => {
    await assertHidden('erin', '/invoices/501', 'INV-501');
  });

  test('a billing admin cannot read an invoice in another tenant', async () => {
    await assertHidden('cara', '/invoices/601', 'INV-601');
  });

  test("a member's list leaves out colleagues' invoices", async () => {
    assert.deepEqual(await listedIds('alice'), [501, 503]);
  });

  test("a billing admin's list stays inside their tenant", async () => {
    assert.deepEqual(await listedIds('cara'), [501, 502, 503]);
  });

  test("an export that includes a colleague's invoice is refused whole", async () => {
    await assertHidden('alice', '/invoices/export?ids=501,502', 'INV-502');
  });

  test("a billing admin's export that reaches into another tenant is refused whole", async () => {
    await assertHidden('cara', '/invoices/export?ids=502,601', 'INV-601');
  });
});

Pitfalls

  • invoice.owner_id === user.id || user.role === 'billing_admin' lets a billing admin of any tenant read every tenant's invoices. That is this card's wrong implementation, and every test inside one tenant passes against it.
  • Mixing && and || without parentheses does the same: sameTenant && isOwner || isAdmin parses as (sameTenant && isOwner) || isAdmin. Parenthesize the role branch in JavaScript and in SQL, where AND also binds tighter than OR.
  • Filtering in JavaScript after an unscoped SELECT loads other tenants' rows into memory and breaks LIMIT, OFFSET and counts. Put the scope in the WHERE clause.
  • A tenant ID from a header such as X-Tenant-ID, a subdomain or the URL is only a selector. If you accept one, check that the signed-in user belongs to that tenant before using it.
  • Random IDs such as UUIDs make guessing harder, but leaked links, logs and browser history still expose them; the check is still required.
  • A response cache keyed only by URL can hand one user's invoice to another. Put the user or the scope in the cache key, or authorize before reading the cache.
  • node:sqlite binds booleans only from Node.js 26.8.0; on older versions pass 1 or 0 for the admin flag, as this card does.

Verify the fix

  1. Run the tests in all three modes: IMPL=fixed must pass; IMPL=vulnerable and IMPL=wrong must fail (node ../../run-all.mjs missing-authorization/node-express does this for you).
  2. Against your own API, sign in as two members of one tenant and as a billing admin of another tenant, then replay each other's invoice IDs on the detail, list and export routes. Every request that crosses a boundary must get a 404 or only in-scope rows.
  3. Do not expect a scanner to find this: no Semgrep registry rule or CodeQL query models object-level authorization for Express, and Semgrep's OWASP Top 10 pack reported nothing on the vulnerable code. The regression tests are the control.

Scanner rules

What each rule reported when it was run on this card's code, or what its source shows it covers where it was not run. A rule that exists for the weakness does not always catch it.

Scanner rules for Fix missing object-level authorization (IDOR) in Node.js and Express
ToolRuleWhat it found here
Semgrepp/owasp-top-tenNo findings on app.js or impl/*.js: the vulnerable, fixed and wrong code look the same to every rule in these packs.Checked: Semgrep 1.180.0 (CE)

No rule targets object-level authorization in JavaScript. In semgrep-rules develop at 9bbf021 the only CWE-639 rule is Rails' check-unscoped-find, and the CWE-862 and CWE-863 rules cover C# MVC controllers, Apex, Dockerfiles, web.xml and AI-assistant settings. In github/codeql main at b5b8165, the JavaScript queries tagged CWE-639 or CWE-862 check CORS credentials and empty passwords in configuration files. eslint-plugin-security has no authorization rule either, so the regression tests are the control.

Libraries and versions

Libraries and versions
ComponentVersion
LanguageJavaScript (ES modules)
RuntimeNode.js 26.11.1 (node:sqlite needs 22.13.0 or later without a flag)
FrameworkExpress 5.3.0
express5.3.0
node:sqlite (built in)Node.js 26.11.1; SQLite 3.53.4 at run time

Run the tests

Download the project, install its dependencies once (this step needs the network), then run the tests against each implementation from the missing-authorization/node-expressfolder.

Shell
npm ci
Shell
# Expected: every test passes
IMPL=fixed npm test --silent

# Expected: an exploit test fails
IMPL=vulnerable npm test --silent

# Expected: a test fails
IMPL=wrong npm test --silent

Each run writes a JUnit report to reports/junit.xml. From the top folder of the download, node run-all.mjs missing-authorization/node-express runs all three modes and checks the expected results.

Download the project ZIP, 25 KB

Verified

Machine
macOS 27.0 (arm64)
Toolchain
  • node 26.11.1
  • npm 11.20.0
  • sqlite 3.53.4
Matrix
fixed: 15 of 15 tests pass; vulnerable: 6 of 15 tests fail (6 exploit tests); wrong: 3 of 15 tests fail (3 exploit tests)
Test results
IMPL=fixed 15/15 pass; IMPL=vulnerable 6/15 fail (6 exploit); IMPL=wrong 3/15 fail (3 exploit); run offline in a no-network sandbox, after installs with warm and with empty package caches

Versions and results hold for the toolchain above. The card's tests are the check: run them with your own versions before relying on the fix.

References

  1. OWASP API Security Top 10 2023: API1 Broken Object Level Authorization
  2. OWASP Insecure Direct Object Reference Prevention Cheat Sheet
  3. OWASP Authorization Cheat Sheet
  4. OWASP Multi-Tenant Security Cheat Sheet
  5. Express: Writing middleware
  6. Node.js v26.11.1: SQLite, statement.all() parameter binding
  7. Node.js v26 (latest): SQLite type conversion (booleans bound from v26.8.0)
  8. SQLite: json_each() table-valued function

Find your next answer

Type to search. Search runs in your browser.

Searches run in your browser, but the host can infer them from the index files they load. The address shows your search so you can bookmark or share it.