From b5450c3ba6cabbddfc4c136e3111473c25004354 Mon Sep 17 00:00:00 2001 From: Steve <263163432+mptfire@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:12:50 +0200 Subject: [PATCH 1/2] fix(backup): decrypt seed words before restoring account storage Prepared by Codex security review. Original local review commit: 908ded786de8fb3c17bbe688e7a2d33471acae18 --- .../service/AccountExportService.kt | 14 ++++++- .../service/AccountExportSeedRestoreTest.kt | 38 +++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 app/src/test/java/com/greenart7c3/nostrsigner/service/AccountExportSeedRestoreTest.kt diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/AccountExportService.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/AccountExportService.kt index 4a465505..e9f11098 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/AccountExportService.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/AccountExportService.kt @@ -16,6 +16,7 @@ import com.vitorpamplona.quartz.nip01Core.core.toHexKey import com.vitorpamplona.quartz.nip01Core.crypto.KeyPair import com.vitorpamplona.quartz.nip01Core.crypto.Nip01Crypto import com.vitorpamplona.quartz.nip01Core.signers.NostrSignerInternal +import com.vitorpamplona.quartz.nip06KeyDerivation.Nip06 import com.vitorpamplona.quartz.nip19Bech32.toNpub import com.vitorpamplona.quartz.nip49PrivKeyEnc.Nip49 import com.vitorpamplona.quartz.utils.Hex @@ -63,8 +64,10 @@ object AccountExportService { didBackup = accountData.didBackup, signer = NostrSignerInternal(KeyPair(privKey.hexToByteArray())), ) - LocalPreferences.switchToAccount(Amber.instance, accountData.npub) - LocalPreferences.updatePrefsForLogin(Amber.instance, account, hexKey.toHexKey(), privKey, accountData.seedWords) + require(accountData.npub == account.npub) { "Backup account does not match its encrypted private key" } + val seedWords = decryptExportedSeedWords(account, accountData.seedWords) + LocalPreferences.switchToAccount(Amber.instance, account.npub) + LocalPreferences.updatePrefsForLogin(Amber.instance, account, hexKey.toHexKey(), privKey, seedWords) } catch (e: Exception) { if (e is CancellationException) throw e @@ -81,6 +84,13 @@ object AccountExportService { } } + internal suspend fun decryptExportedSeedWords(account: Account, encryptedSeedWords: String?): String? { + if (encryptedSeedWords.isNullOrBlank()) return null + val seedWords = account.nip44Decrypt(encryptedSeedWords, account.hexKey) + require(Nip06().isValidMnemonic(seedWords)) { "Backup seed words are invalid" } + return seedWords + } + /** * Export each account as a separate (npub, ncryptsec) pair. * Callers write each pair to its own file named "{npub}.ncryptsec". diff --git a/app/src/test/java/com/greenart7c3/nostrsigner/service/AccountExportSeedRestoreTest.kt b/app/src/test/java/com/greenart7c3/nostrsigner/service/AccountExportSeedRestoreTest.kt new file mode 100644 index 00000000..fcf752bf --- /dev/null +++ b/app/src/test/java/com/greenart7c3/nostrsigner/service/AccountExportSeedRestoreTest.kt @@ -0,0 +1,38 @@ +package com.greenart7c3.nostrsigner.service + +import com.greenart7c3.nostrsigner.models.Account +import io.mockk.coEvery +import io.mockk.every +import io.mockk.mockk +import kotlinx.coroutines.runBlocking +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertThrows +import org.junit.Test + +class AccountExportSeedRestoreTest { + private val words = "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" + + @Test + fun `restore returns decrypted words instead of storing ciphertext as a mnemonic`() = runBlocking { + val account = mockk() + every { account.hexKey } returns "owner" + coEvery { account.nip44Decrypt("encrypted-seed", "owner") } returns words + assertEquals(words, AccountExportService.decryptExportedSeedWords(account, "encrypted-seed")) + } + + @Test + fun `absent seed does not overwrite existing words`() = runBlocking { + assertNull(AccountExportService.decryptExportedSeedWords(mockk(), "")) + } + + @Test + fun `invalid decrypted words fail before account storage`() { + val account = mockk() + every { account.hexKey } returns "owner" + coEvery { account.nip44Decrypt("encrypted-seed", "owner") } returns "not a mnemonic" + assertThrows(IllegalArgumentException::class.java) { + runBlocking { AccountExportService.decryptExportedSeedWords(account, "encrypted-seed") } + } + } +} From 7e96ce7d6b899100862c5c334195e4692a75d086 Mon Sep 17 00:00:00 2001 From: Codex Security Review Date: Tue, 13 Oct 2026 22:25:43 +0200 Subject: [PATCH 2/2] test(backup): cover the backup-identity mismatch requirement The npub-vs-restored-key require is the import gate that stops a backup whose metadata names a different identity; extract it next to decryptExportedSeedWords and cover both branches. --- .../nostrsigner/service/AccountExportService.kt | 6 +++++- .../service/AccountExportSeedRestoreTest.kt | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/greenart7c3/nostrsigner/service/AccountExportService.kt b/app/src/main/java/com/greenart7c3/nostrsigner/service/AccountExportService.kt index e9f11098..28bd78f6 100644 --- a/app/src/main/java/com/greenart7c3/nostrsigner/service/AccountExportService.kt +++ b/app/src/main/java/com/greenart7c3/nostrsigner/service/AccountExportService.kt @@ -64,7 +64,7 @@ object AccountExportService { didBackup = accountData.didBackup, signer = NostrSignerInternal(KeyPair(privKey.hexToByteArray())), ) - require(accountData.npub == account.npub) { "Backup account does not match its encrypted private key" } + requireBackupIdentityMatches(accountData.npub, account) val seedWords = decryptExportedSeedWords(account, accountData.seedWords) LocalPreferences.switchToAccount(Amber.instance, account.npub) LocalPreferences.updatePrefsForLogin(Amber.instance, account, hexKey.toHexKey(), privKey, seedWords) @@ -84,6 +84,10 @@ object AccountExportService { } } + internal fun requireBackupIdentityMatches(metadataNpub: String, account: Account) { + require(metadataNpub == account.npub) { "Backup account does not match its encrypted private key" } + } + internal suspend fun decryptExportedSeedWords(account: Account, encryptedSeedWords: String?): String? { if (encryptedSeedWords.isNullOrBlank()) return null val seedWords = account.nip44Decrypt(encryptedSeedWords, account.hexKey) diff --git a/app/src/test/java/com/greenart7c3/nostrsigner/service/AccountExportSeedRestoreTest.kt b/app/src/test/java/com/greenart7c3/nostrsigner/service/AccountExportSeedRestoreTest.kt index fcf752bf..942e4c39 100644 --- a/app/src/test/java/com/greenart7c3/nostrsigner/service/AccountExportSeedRestoreTest.kt +++ b/app/src/test/java/com/greenart7c3/nostrsigner/service/AccountExportSeedRestoreTest.kt @@ -35,4 +35,14 @@ class AccountExportSeedRestoreTest { runBlocking { AccountExportService.decryptExportedSeedWords(account, "encrypted-seed") } } } + + @Test + fun `metadata npub must match the restored key identity`() { + val account = mockk() + every { account.npub } returns "npub-restored" + assertThrows(IllegalArgumentException::class.java) { + AccountExportService.requireBackupIdentityMatches("npub-metadata", account) + } + AccountExportService.requireBackupIdentityMatches("npub-restored", account) + } }