aiolabs/nsecbunkerd
9
0
Fork 0
Code Issues 17 Pull requests Projects Releases Packages Wiki Activity Actions

NIP-46 transport matches responses by request id only — no binding to the peer the request was sent to #59

New issue
Open
opened 2026-10-09 17:18:10 +00:00 by padreug · 0 comments
padreug commented 2026-10-09 17:18:10 +00:00
Owner
Copy link

src/daemon/nip46/transport.ts:82-84 resolves this.pending.get(body.id) for any verified, decryptable event carrying a matching id, regardless of event.pubkey; parseEnvelope has remotePubkey in hand but onEvent never compares it to the admin pubkey the sendRequest (:125-149) was addressed to. admin/index.ts:452-468 then feeds the first answer into requestPermissionResponse, whose always branch writes a permanent allowAllRequestsFromKey grant. Same shape on the client: src/nip46-client.ts:52-59 decrypts under the sender's key and resolves by msg.id, and :63-66 shows any id-matching sender's auth_url. Request ids are 10 base36 chars from Math.random (transport.ts:133).

Impact: a non-admin who can predict or learn a pending ACL request id can approve a signing request the operator never saw, within the 10 s window; on the client, any pubkey can forge an ack/signed-event/phishing auth_url. Practical difficulty is real (the id travels only inside a nip44 envelope to the admin), which is why the transport review parked this as low (#52 CL-3/CS-5, AD-5/CS-1); the sandbox rates it high because the fix is a one-line check on an authorization path. Decide severity, but fix it either way.

Fix direction: store the expected peer pubkey alongside each pending entry and drop responses whose event.pubkey differs; in Nip46Client require event.pubkey === this.remotePubkey; generate ids from crypto.randomBytes.

Found during reforge run #1 (sandbox nsecbunkerd#18). Supersedes the deferred items in #52.

`src/daemon/nip46/transport.ts:82-84` resolves `this.pending.get(body.id)` for any verified, decryptable event carrying a matching `id`, regardless of `event.pubkey`; `parseEnvelope` has `remotePubkey` in hand but `onEvent` never compares it to the admin pubkey the `sendRequest` (`:125-149`) was addressed to. `admin/index.ts:452-468` then feeds the first answer into `requestPermissionResponse`, whose `always` branch writes a permanent `allowAllRequestsFromKey` grant. Same shape on the client: `src/nip46-client.ts:52-59` decrypts under the sender's key and resolves by `msg.id`, and `:63-66` shows any id-matching sender's `auth_url`. Request ids are 10 base36 chars from `Math.random` (`transport.ts:133`). Impact: a non-admin who can predict or learn a pending ACL request id can approve a signing request the operator never saw, within the 10 s window; on the client, any pubkey can forge an `ack`/signed-event/phishing `auth_url`. Practical difficulty is real (the id travels only inside a nip44 envelope to the admin), which is why the transport review parked this as low (#52 CL-3/CS-5, AD-5/CS-1); the sandbox rates it high because the fix is a one-line check on an authorization path. Decide severity, but fix it either way. Fix direction: store the expected peer pubkey alongside each `pending` entry and drop responses whose `event.pubkey` differs; in `Nip46Client` require `event.pubkey === this.remotePubkey`; generate ids from `crypto.randomBytes`. Found during reforge run #1 (sandbox nsecbunkerd#18). Supersedes the deferred items in #52.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
master
dev
docs-migration-runbook
issue-9-fix-create-new-key-race
No results found.
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
aiolabs/nsecbunkerd#59
No description provided.