fix(activities): close existence oracle on PATCH /:id/sort
The sort endpoint validated existence with a bare `SELECT 1 FROM activities WHERE id = ?`, ignoring visibility. A logged-in attacker could PATCH /sort with any UUID and distinguish "private id exists, owned by someone else" (200) from "id doesn't exist" (404), letting them enumerate private activity ids. Apply the same visibility filter as GET /:id, toggleDone, and toggleFiling: private requires owner; friends requires mutual-friend + no block in either direction; hidden rows return 404, not 403. Regression test added in tests/activities.test.ts. Surfaced by /audit security (HIGH severity).
This commit is contained in:
parent
09a9e3742c
commit
0e5bf0a035
2 changed files with 61 additions and 7 deletions
|
|
@ -556,13 +556,32 @@ activitiesRoutes.patch('/:id/sort', requireAuth, async (c) => {
|
|||
return c.json({ error: 'missing:position' }, 400);
|
||||
}
|
||||
const db = getDb();
|
||||
// Confirm the activity exists AND the viewer can see it. Anything else is
|
||||
// a 404 — we don't want callers persisting positions for activities they
|
||||
// can't see, even though it wouldn't surface anywhere visible.
|
||||
const visible = db
|
||||
.prepare('SELECT 1 FROM activities WHERE id = ?')
|
||||
.get(id);
|
||||
if (!visible) return c.json({ error: 'not_found' }, 404);
|
||||
// Apply the same visibility filter as GET /:id and the list endpoint so
|
||||
// sort doesn't double as an existence oracle for private / friends-only
|
||||
// activity ids. Hidden rows return 404 (not 403). Earlier this endpoint
|
||||
// only did a bare `SELECT 1 FROM activities WHERE id = ?` — surfaced by
|
||||
// /audit security as a HIGH severity finding.
|
||||
const row = db
|
||||
.prepare('SELECT visibility, owner_id FROM activities WHERE id = ?')
|
||||
.get(id) as { visibility: Visibility; owner_id: string } | null;
|
||||
if (!row) return c.json({ error: 'not_found' }, 404);
|
||||
if (row.visibility === 'private' && row.owner_id !== userId) {
|
||||
return c.json({ error: 'not_found' }, 404);
|
||||
}
|
||||
if (row.visibility === 'friends' && row.owner_id !== userId) {
|
||||
const isFriend = !!db
|
||||
.prepare('SELECT 1 FROM friends WHERE owner_id = ? AND friend_id = ?')
|
||||
.get(row.owner_id, userId);
|
||||
if (!isFriend) return c.json({ error: 'not_found' }, 404);
|
||||
const blocked = !!db
|
||||
.prepare(`
|
||||
SELECT 1 FROM user_blocks
|
||||
WHERE (blocker_id = ? AND blocked_id = ?)
|
||||
OR (blocker_id = ? AND blocked_id = ?)
|
||||
`)
|
||||
.get(row.owner_id, userId, userId, row.owner_id);
|
||||
if (blocked) return c.json({ error: 'not_found' }, 404);
|
||||
}
|
||||
|
||||
db.prepare(`
|
||||
INSERT INTO user_activity_sort (user_id, activity_id, position) VALUES (?, ?, ?)
|
||||
|
|
|
|||
|
|
@ -384,6 +384,41 @@ describe('per-user sort order', () => {
|
|||
expect(res.status).toBe(400);
|
||||
}
|
||||
});
|
||||
|
||||
ttest('PATCH /sort does not double as an existence oracle for hidden rows', async () => {
|
||||
// Regression: the endpoint previously did a bare `SELECT 1 FROM activities`
|
||||
// without visibility scoping, so a logged-in attacker could distinguish
|
||||
// "private id exists, owned by someone else" (200 ok) from "id doesn't
|
||||
// exist" (404). /audit security flagged this as HIGH.
|
||||
const [owner, attacker] = await Promise.all([
|
||||
signupAndGetCookie(ctx, 'sort-oracle-owner@test.invalid'),
|
||||
signupAndGetCookie(ctx, 'sort-oracle-att@test.invalid'),
|
||||
]);
|
||||
const priv = await createActivity(ctx, owner.cookie, {
|
||||
visibility: 'private',
|
||||
ciphertext: 'AAAA',
|
||||
nonce: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA',
|
||||
} as never);
|
||||
|
||||
// Attacker tries to sort someone else's private row → must get 404
|
||||
// (same status as for a truly nonexistent id).
|
||||
const sortOther = await req(ctx, 'PATCH', `/api/activities/${priv.id}/sort`, {
|
||||
cookie: attacker.cookie, body: { position: 0 },
|
||||
});
|
||||
expect(sortOther.status).toBe(404);
|
||||
|
||||
// And for a truly nonexistent id.
|
||||
const sortMissing = await req(ctx, 'PATCH', '/api/activities/nope-nope-nope/sort', {
|
||||
cookie: attacker.cookie, body: { position: 0 },
|
||||
});
|
||||
expect(sortMissing.status).toBe(404);
|
||||
|
||||
// Owner can still sort their own private row.
|
||||
const sortOwn = await req(ctx, 'PATCH', `/api/activities/${priv.id}/sort`, {
|
||||
cookie: owner.cookie, body: { position: 0 },
|
||||
});
|
||||
expect(sortOwn.status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
describe('owner_display fallback chain (no email leak)', () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue