Skip to content

Commit 704c359

Browse files
committed
fix: harden bunker, key storage, permissions, and sync (audit pass)
Findings from a focused security/correctness audit of the touched subsystems: - Bunker exposure (critical): gate bunkerServer.start/stop behind isExtensionSender. They were callable from any web page via window.nostr.nip46.startBunker(), which returned a secret-bearing bunker:// string granting unlimited remote signing of the user's key. - Bunker connect: fail closed when the server has no secret, instead of authenticating any client. - savePrivateKey (critical): when encryption is enabled but the session is locked (no session key), throw instead of silently persisting the key as plaintext into a vault the user believes is encrypted. - Stale session-key cache (critical): getPlaintextPrivKey now verifies the index-cached key derives to the profile's pubkey before use. Deleting a profile shifts indices without updating the index-keyed cache, so a sign could otherwise use a different identity's key. - setPermission: use index == null (not !index) so per-site grants for profile 0 target profile 0, not whatever profile is active. - content.js permission sheet: always reply (catch errors) so a failed sheet can't hang the background ask() until its 10s timeout silently denies. - Sync version compare: numeric semver compare so 1.10.0 isn't treated as older than 1.9.0 by string comparison. Tests: add compareSemver coverage (+3).
1 parent 7d18b29 commit 704c359

19 files changed

Lines changed: 172 additions & 38 deletions

distros/safari/api-keys/api-keys.build.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

distros/safari/background.build.js

Lines changed: 32 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

distros/safari/background.js

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,11 @@ const SENSITIVE_KINDS = new Set([
374374
'setPassword', 'changePassword', 'removePassword', 'resetAllData',
375375
'setAutoLockTimeout', 'setNostrAccessWhileLocked', 'setBlockCrossOriginFrames',
376376
'backup.export', 'backup.import', 'unlock',
377+
// Starting/stopping the NIP-46 bunker exposes the user's key for remote
378+
// signing and returns the secret-bearing connection string. It must be a
379+
// deliberate in-extension action — never triggerable by a web page via
380+
// window.nostr.nip46.startBunker().
381+
'bunkerServer.start', 'bunkerServer.stop',
377382
]);
378383

379384
function isExtensionSender(sender) {
@@ -1601,9 +1606,16 @@ async function savePrivateKey([index, privKey]) {
16011606
const pubKey = getPublicKeySync(hexKey);
16021607
profiles[index].pubKey = pubKey;
16031608

1604-
// If encryption is active, re-encrypt the new key using the session key
1609+
// If encryption is active, re-encrypt the new key using the session key.
16051610
const encrypted = await isEncrypted();
1606-
if (encrypted && sessionCryptoKey) {
1611+
if (encrypted) {
1612+
// Encryption is on but there's no live session key (locked, or the MV3
1613+
// worker was evicted and lost it). Refuse rather than fall through and
1614+
// persist the key as PLAINTEXT into a vault the user believes is
1615+
// encrypted. The caller surfaces this as a save error.
1616+
if (!sessionCryptoKey) {
1617+
throw new Error('Extension is locked — unlock before saving a key');
1618+
}
16071619
profiles[index].privKey = await encryptWithKey(hexKey, sessionCryptoKey, sessionKeySalt);
16081620
sessionKeys.set(index, hexKey);
16091621
} else {
@@ -1662,7 +1674,19 @@ async function getPlaintextPrivKey(index, profile) {
16621674
if (isEncryptedBlob(profile.privKey)) {
16631675
// Key is encrypted — must use session cache
16641676
if (sessionKeys.has(index)) {
1665-
return sessionKeys.get(index);
1677+
const cached = sessionKeys.get(index);
1678+
// Guard against a stale cache entry. sessionKeys is keyed by profile
1679+
// index, but deleting a profile shifts every later index down by one
1680+
// without updating this in-memory map — so sessionKeys.get(index)
1681+
// could be a DIFFERENT identity's key. Verify the cached key actually
1682+
// derives to this profile's pubkey before returning it; otherwise we
1683+
// would sign with the wrong key. If the profile has no cached pubkey
1684+
// we can't validate, so fall back to the legacy behaviour.
1685+
if (!profile.pubKey || getPublicKeySync(cached) === profile.pubKey) {
1686+
return cached;
1687+
}
1688+
// Stale entry — drop it and treat this profile as locked.
1689+
sessionKeys.delete(index);
16661690
}
16671691
throw new Error('Extension is locked — cannot access private key');
16681692
}

distros/safari/content.build.js

Lines changed: 2 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

distros/safari/content.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,14 @@ function dismissLockedSheet() {
353353
// Listen for requests from background
354354
api.runtime.onMessage.addListener((message, sender, sendResponse) => {
355355
if (message.kind === 'showPermissionSheet') {
356-
showPermissionSheet(message.host, message.permissionKind, message.queuePosition, message.queueTotal).then(result => {
357-
sendResponse(result);
358-
});
356+
// Always reply, even on failure. This handler holds the channel open
357+
// (return true); if showPermissionSheet ever rejects/throws and we never
358+
// call sendResponse, the background's ask() await stalls until its 10s
359+
// timeout silently DENIES the request. Replying null lets it fall back.
360+
Promise.resolve()
361+
.then(() => showPermissionSheet(message.host, message.permissionKind, message.queuePosition, message.queueTotal))
362+
.then(result => sendResponse(result))
363+
.catch(() => sendResponse(null));
359364
return true; // Keep channel open for async response
360365
}
361366
if (message.kind === 'showLockedSheet') {

distros/safari/event_history/event_history.build.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

distros/safari/nostr-keys/nostr-keys.build.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

distros/safari/options.build.js

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

distros/safari/permission/permission.build.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

distros/safari/popup.build.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)