fix(security): POST /api/auth/update trusted a client-supplied pubkey (IDOR)
Found live while wiring the "existing bot" AI-config settings UI: this route parsed `pubkey` from the request body and used it directly to select which bot row to update, with no check that it matched the caller's actual authenticated identity. Any unauthenticated caller could POST an arbitrary victim's pubkey plus a malicious webhookUrl, profilePicUrl, or customization payload and silently hijack that bot (e.g. redirect its webhook to an attacker-controlled endpoint). Contrast with GET /me and POST /regenerate-secret, which both correctly derive pubkey from the verified JWT via extractPubkeyFromAuth and never trust a client-claimed identity — this was the one route that didn't follow that pattern. Fixed by deriving pubkey from the JWT exclusively; updateBotSchema no longer declares a pubkey field at all (was the only schema-level signal that the vulnerable code path existed). Frontend callers updated to stop sending a pubkey they no longer need. Added a regression suite (auth-update.test.ts) covering: 401 with no/garbage auth, hijack-attempt-via-body-pubkey now 404s and leaves the victim's row untouched, and legitimate self-updates still work when the body happens to carry an unrelated pubkey field (ignored, not trusted). Full server suite: 829/829 passing. tsc --noEmit clean (server + frontend). Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -380,7 +380,7 @@ export function useNostr() {
|
||||
const res = await authFetch('/api/auth/update', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ pubkey: pubkey.value, customization }),
|
||||
body: JSON.stringify({ customization }),
|
||||
})
|
||||
|
||||
if (!res.ok) {
|
||||
@@ -404,7 +404,7 @@ export function useNostr() {
|
||||
const res = await authFetch('/api/auth/update', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ pubkey: pubkey.value, webhookUrl: newUrl }),
|
||||
body: JSON.stringify({ webhookUrl: newUrl }),
|
||||
})
|
||||
|
||||
const data = await res.json()
|
||||
|
||||
@@ -132,14 +132,22 @@ describe('registerHumanSchema', () => {
|
||||
})
|
||||
|
||||
describe('updateBotSchema', () => {
|
||||
const base = { pubkey: 'a'.repeat(64) }
|
||||
it('accepts pubkey only (no updates)', () => { expect(updateBotSchema.safeParse(base).success).toBe(true) })
|
||||
const base = {}
|
||||
it('accepts empty body (no updates)', () => { expect(updateBotSchema.safeParse(base).success).toBe(true) })
|
||||
it('accepts webhook update', () => {
|
||||
expect(updateBotSchema.safeParse({ ...base, webhookUrl: 'https://new.com/hook' }).success).toBe(true)
|
||||
})
|
||||
it('rejects file:// webhook', () => {
|
||||
expect(updateBotSchema.safeParse({ ...base, webhookUrl: 'file:///etc/passwd' }).success).toBe(false)
|
||||
})
|
||||
// SECURITY REGRESSION: pubkey must never be a schema field here. POST
|
||||
// /api/auth/update derives identity from the verified JWT
|
||||
// (extractPubkeyFromAuth), not from client body — see server/src/routes/auth.ts.
|
||||
// A pubkey field in this schema previously let an unauthenticated caller
|
||||
// claim any bot as their own and hijack its webhook/customization.
|
||||
it('does not declare a pubkey field (identity comes from the JWT, not the body)', () => {
|
||||
expect('pubkey' in updateBotSchema.shape).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
// --- Fight schemas ---
|
||||
|
||||
@@ -42,8 +42,13 @@ export const registerHumanSchema = z.object({
|
||||
avatarSeed: z.string().min(1).max(50).optional(),
|
||||
})
|
||||
|
||||
// pubkey is intentionally NOT part of this schema: POST /api/auth/update
|
||||
// derives the caller's identity from their verified JWT (extractPubkeyFromAuth),
|
||||
// never from the request body — a client-supplied pubkey here would let any
|
||||
// caller act as any other bot owner. Kept accepting-but-ignoring the field
|
||||
// would be more confusing than just not declaring it; the frontend no longer
|
||||
// sends it either.
|
||||
export const updateBotSchema = z.object({
|
||||
pubkey: pubkeySchema,
|
||||
webhookUrl: httpUrlSchema.max(2048).optional(),
|
||||
profilePicUrl: httpUrlSchema.max(2048).optional(),
|
||||
customization: z.record(z.string(), z.unknown()).optional().nullable(),
|
||||
|
||||
@@ -0,0 +1,111 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { Hono } from 'hono'
|
||||
import { authRouter } from './auth.js'
|
||||
import { generateSecretKey, getPublicKey } from 'nostr-tools'
|
||||
import { createJwt } from '../middleware/jwt.js'
|
||||
import { db, schema } from '../db/index.js'
|
||||
import { eq } from 'drizzle-orm'
|
||||
import { createHash, randomBytes } from 'crypto'
|
||||
import { nanoid } from 'nanoid'
|
||||
|
||||
const app = new Hono()
|
||||
app.route('/api/auth', authRouter)
|
||||
|
||||
async function seedBot(pubkey: string, overrides: Partial<typeof schema.bots.$inferInsert> = {}) {
|
||||
const id = overrides.id || nanoid(12)
|
||||
const secret = randomBytes(32).toString('hex')
|
||||
await db.insert(schema.bots).values({
|
||||
id,
|
||||
name: overrides.name || `t${Date.now().toString(36).slice(-8)}`,
|
||||
webhookUrl: overrides.webhookUrl || 'http://poll.local/',
|
||||
avatarSeed: overrides.avatarSeed || 'seed',
|
||||
archetype: overrides.archetype || 'standard',
|
||||
secretHash: createHash('sha256').update(secret).digest('hex'),
|
||||
publicKey: pubkey,
|
||||
profilePicUrl: overrides.profilePicUrl ?? null,
|
||||
customization: overrides.customization ?? null,
|
||||
createdAt: overrides.createdAt || new Date().toISOString(),
|
||||
})
|
||||
return id
|
||||
}
|
||||
|
||||
// SECURITY REGRESSION SUITE for POST /api/auth/update.
|
||||
//
|
||||
// This route previously trusted a client-supplied `pubkey` field in the
|
||||
// request body with no verification against the caller's actual identity —
|
||||
// any unauthenticated caller could pass a victim's pubkey and hijack their
|
||||
// bot's webhook/profilePicUrl/customization. Found live during 09-06
|
||||
// (ai-config UI work) by contrast with GET /me and POST /regenerate-secret,
|
||||
// which both correctly derive identity from the verified JWT via
|
||||
// extractPubkeyFromAuth. Fixed to always derive pubkey from the JWT; the
|
||||
// body no longer even has a pubkey field (see updateBotSchema).
|
||||
describe('POST /api/auth/update — identity comes from the JWT, not the body', () => {
|
||||
it('rejects an unauthenticated request (no Authorization header)', async () => {
|
||||
const res = await app.request('/api/auth/update', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ customization: { archetype: 'tank' } }),
|
||||
})
|
||||
expect(res.status).toBe(401)
|
||||
})
|
||||
|
||||
it('rejects a garbage/tampered Bearer token', async () => {
|
||||
const res = await app.request('/api/auth/update', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', Authorization: 'Bearer not-a-real-jwt' },
|
||||
body: JSON.stringify({ customization: { archetype: 'tank' } }),
|
||||
})
|
||||
expect(res.status).toBe(401)
|
||||
})
|
||||
|
||||
it('cannot hijack another bot by passing its pubkey in the body', async () => {
|
||||
const victimSk = generateSecretKey()
|
||||
const victimPk = getPublicKey(victimSk)
|
||||
const victimId = await seedBot(victimPk, { webhookUrl: 'http://victim.local/original' })
|
||||
|
||||
// Attacker has their own valid session (their own JWT) but a DIFFERENT
|
||||
// bot — no bot row at all, in this case.
|
||||
const attackerPk = getPublicKey(generateSecretKey())
|
||||
const attackerToken = createJwt(attackerPk)
|
||||
|
||||
const res = await app.request('/api/auth/update', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${attackerToken}` },
|
||||
// Old exploit shape: claim to be the victim via a body field.
|
||||
body: JSON.stringify({ pubkey: victimPk, webhookUrl: 'http://poll.local/attacker-controlled' }),
|
||||
})
|
||||
|
||||
// Attacker has no bot of their own -> 404, NOT a successful update of
|
||||
// the victim's bot.
|
||||
expect(res.status).toBe(404)
|
||||
|
||||
const rows = await db.select({ webhookUrl: schema.bots.webhookUrl })
|
||||
.from(schema.bots)
|
||||
.where(eq(schema.bots.id, victimId))
|
||||
.limit(1)
|
||||
expect(rows[0].webhookUrl).toBe('http://victim.local/original')
|
||||
})
|
||||
|
||||
it('updates the caller\'s own bot, derived from their JWT, ignoring a body pubkey', async () => {
|
||||
const sk = generateSecretKey()
|
||||
const pk = getPublicKey(sk)
|
||||
const id = await seedBot(pk, { webhookUrl: 'http://poll.local/' })
|
||||
const token = createJwt(pk)
|
||||
|
||||
const someoneElsesPk = getPublicKey(generateSecretKey())
|
||||
const res = await app.request('/api/auth/update', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${token}` },
|
||||
// Even if a stale client sends an unrelated pubkey, the server must
|
||||
// still act on the JWT-derived identity, not this field.
|
||||
body: JSON.stringify({ pubkey: someoneElsesPk, customization: { archetype: 'shark' } }),
|
||||
})
|
||||
expect(res.status).toBe(200)
|
||||
|
||||
const rows = await db.select({ customization: schema.bots.customization })
|
||||
.from(schema.bots)
|
||||
.where(eq(schema.bots.id, id))
|
||||
.limit(1)
|
||||
expect(JSON.parse(rows[0].customization || '{}').archetype).toBe('shark')
|
||||
})
|
||||
})
|
||||
@@ -324,11 +324,25 @@ authRouter.post('/register-human', rateLimit(600_000, 10), async (c) => {
|
||||
|
||||
// Update bot webhook and/or customization (requires pubkey match)
|
||||
authRouter.post('/update', rateLimit(60_000, 10), async (c) => {
|
||||
// SECURITY: pubkey MUST come from the verified JWT, never from the request
|
||||
// body. This handler previously trusted a client-supplied `pubkey` field
|
||||
// with no cross-check against the caller's actual authenticated identity —
|
||||
// any unauthenticated caller could POST an arbitrary victim's pubkey plus
|
||||
// a malicious webhookUrl/profilePicUrl/customization and silently hijack
|
||||
// that bot (e.g. redirect its webhook to an attacker-controlled endpoint).
|
||||
// Found live during the ai-config UI work (09-06) by contrast with
|
||||
// /regenerate-secret and GET /me, which both correctly derive pubkey from
|
||||
// extractPubkeyFromAuth and never trust a client-claimed identity.
|
||||
const pubkey = extractPubkeyFromAuth(c.req.header('Authorization'))
|
||||
if (!pubkey) {
|
||||
return c.json({ error: 'Authentication required.' }, 401)
|
||||
}
|
||||
|
||||
const parsed = updateBotSchema.safeParse(await c.req.json().catch(() => ({})))
|
||||
if (!parsed.success) {
|
||||
return c.json({ error: 'Invalid pubkey.' }, 400)
|
||||
return c.json({ error: 'Invalid request body.' }, 400)
|
||||
}
|
||||
const { pubkey, webhookUrl, profilePicUrl, customization: rawCustomization } = parsed.data
|
||||
const { webhookUrl, profilePicUrl, customization: rawCustomization } = parsed.data
|
||||
|
||||
const rows = await db.select({ id: schema.bots.id, customization: schema.bots.customization })
|
||||
.from(schema.bots)
|
||||
|
||||
Reference in New Issue
Block a user