mirror of
https://github.com/simplex-chat/simplex-chat.git
synced 2026-09-27 17:58:47 +00:00
core: store the seed as bytes, and bind one account per profile
On Postgres a bare ByteString binds as a text literal, so entropy with a backslash or a high bit was rejected and entropy with a zero byte was truncated: the BIP-39 test vector stored as an empty seed. Blob columns go through DB.Binary here, as every other one does. /_wallet bind with no account moved a profile that already had one to a fresh account, abandoning the old one without saying so. It refuses now; moving is asked for by number. The counter could walk past 2^31, where BIP-32 hardening folds every index back onto a low one, so a profile handed 2147483648 derived account 0's keys and printed account 0's path. Binding refuses once the counter is there, and a very long index is now rejected on its digit count rather than after reading it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rvc3HbiWBTqbAvRT45G5oX
This commit is contained in:
co-authored by
Claude Opus 5
parent
03b6ed9a53
commit
619434626c
@@ -1503,8 +1503,7 @@ processChatCommand cxt nm = \case
|
||||
pure $ CRWallet user True paths profiles
|
||||
APIWalletBind acct_ -> withUser $ \user -> do
|
||||
seed <- deviceSeed
|
||||
bound <- withFastStore' $ \db -> bindAccountIndex db user (wsId seed) acct_
|
||||
unless bound $ throwCmdError "another profile uses this account"
|
||||
withFastStore' (\db -> bindAccountIndex db user (wsId seed) acct_) >>= either throwCmdError pure
|
||||
processChatCommand cxt nm APIWallet
|
||||
APIWalletCreate -> withUser $ \_ -> do
|
||||
g <- asks random
|
||||
@@ -6143,10 +6142,12 @@ chatCommandP =
|
||||
quotedP = safeDecodeUtf8 <$> (A.char '"' *> A.takeTill (== '"') <* A.char '"')
|
||||
text1P = safeDecodeUtf8 <$> A.takeTill (== ' ')
|
||||
char_ = optional . A.char
|
||||
-- BIP-32 hardens at 2^31, and Word32 would wrap
|
||||
-- BIP-32 hardens at 2^31, and Word32 would wrap. Digits are counted before
|
||||
-- they are read, as reading a very long number is not free.
|
||||
keyIndexP = do
|
||||
i <- A.decimal :: Parser Integer
|
||||
if i < 0x80000000 then pure (fromIntegral i) else fail "key index too large"
|
||||
ds <- A.takeWhile1 isDigit
|
||||
let i = read (B.unpack ds) :: Integer
|
||||
if B.length ds <= 10 && i < 0x80000000 then pure (fromIntegral i) else fail "key index too large"
|
||||
|
||||
displayNameP :: Parser Text
|
||||
displayNameP = safeDecodeUtf8 <$> displayNameP_
|
||||
|
||||
@@ -73,13 +73,21 @@ createSeed :: DB.Connection -> ByteString -> IO Bool
|
||||
createSeed db entropy =
|
||||
getDeviceSeed db >>= \case
|
||||
Just _ -> pure False
|
||||
Nothing -> True <$ DB.execute db "INSERT INTO wallet_seeds (seed) VALUES (?)" (Only entropy)
|
||||
Nothing -> True <$ DB.execute db "INSERT INTO wallet_seeds (seed) VALUES (?)" (Only $ DB.Binary entropy)
|
||||
|
||||
-- | Without an account the next free one is taken. False if another profile
|
||||
-- holds the account asked for.
|
||||
bindAccountIndex :: DB.Connection -> User -> SeedId -> Maybe AccountIndex -> IO Bool
|
||||
bindAccountIndex db User {userId} sId = \case
|
||||
Nothing -> True <$ (takeAccountIndex db sId >>= bindUser db userId sId)
|
||||
-- | Without an account the next free one is taken, which a profile that has
|
||||
-- one does not need: moving to another account is asked for by number.
|
||||
bindAccountIndex :: DB.Connection -> User -> SeedId -> Maybe AccountIndex -> IO (Either String ())
|
||||
bindAccountIndex db user@User {userId} sId = \case
|
||||
Nothing ->
|
||||
getAccountIndex db user >>= \case
|
||||
Just _ -> pure $ Left "this profile already has an account"
|
||||
Nothing -> do
|
||||
acct <- takeAccountIndex db sId
|
||||
-- BIP-32 hardens at 2^31, and every index above it is the same key again
|
||||
if acct >= 0x80000000
|
||||
then pure $ Left "no free account on this key"
|
||||
else Right () <$ bindUser db userId sId acct
|
||||
Just acct -> do
|
||||
taken <-
|
||||
maybeFirstRow fromOnly $
|
||||
@@ -88,12 +96,12 @@ bindAccountIndex db User {userId} sId = \case
|
||||
"SELECT 1 FROM users WHERE wallet_seed_id = ? AND wallet_account_index = ? AND user_id != ?"
|
||||
(sId, fromIntegral acct :: Int64, userId)
|
||||
case (taken :: Maybe Int64) of
|
||||
Just _ -> pure False
|
||||
Just _ -> pure $ Left "another profile uses this account"
|
||||
Nothing -> do
|
||||
bindUser db userId sId (fromIntegral acct)
|
||||
-- the counter moves past it, so the next profile is not handed the same one
|
||||
setNextAccountIndex db sId (fromIntegral acct + 1)
|
||||
pure True
|
||||
pure $ Right ()
|
||||
|
||||
-- | So two profiles cannot be handed the same account.
|
||||
takeAccountIndex :: DB.Connection -> SeedId -> IO Int64
|
||||
|
||||
@@ -56,6 +56,7 @@ walletTests = do
|
||||
it "exports the secret of any name key" testWalletExportDerivedSecret
|
||||
it "deletes the key, and a key can be imported again" testWalletDelete
|
||||
it "binds a profile to the account it had" testWalletBind
|
||||
it "binds a profile once, and only to an account BIP-32 can harden" testWalletBindLimits
|
||||
it "needs no import when the database was backed up after the key" testWalletBackupAfterKey
|
||||
it "rebinds by index when the database was backed up before the key" testWalletBackupBeforeKey
|
||||
it "discards a key imported before the database is restored" testWalletImportThenRestore
|
||||
@@ -245,3 +246,24 @@ testWalletImportThenRestore ps = withNewTestChat ps "alice" aliceProfile $ \alic
|
||||
alice ##> "/_wallet bind"
|
||||
rows <- nameRows alice
|
||||
map fst rows `shouldBe` ["m/44'/60'/0'/0/0", "m/44'/60'/0'/0/1"]
|
||||
|
||||
testWalletBindLimits :: HasCallStack => TestParams -> IO ()
|
||||
testWalletBindLimits ps = withNewTestChat ps "alice" aliceProfile $ \alice -> do
|
||||
alice ##> ("/_wallet import " <> B.unpack testPhrase)
|
||||
alice <## "no account for this profile"
|
||||
alice ##> "/_wallet bind"
|
||||
_ <- nameRows alice
|
||||
-- a profile that has an account asks for another one by number
|
||||
alice ##> "/_wallet bind"
|
||||
alice <## "bad chat command: this profile already has an account"
|
||||
alice ##> "/create user alisa"
|
||||
showActiveUser alice "alisa"
|
||||
alice ##> "/_wallet bind 2147483647"
|
||||
rows <- nameRows alice
|
||||
alice <## "also on same seed: alice"
|
||||
map fst rows `shouldBe` ["m/44'/60'/2147483647'/0/0", "m/44'/60'/2147483647'/0/1"]
|
||||
-- the counter is past what BIP-32 can harden, where it would repeat account 0
|
||||
alice ##> "/create user carol"
|
||||
showActiveUser alice "carol"
|
||||
alice ##> "/_wallet bind"
|
||||
alice <## "bad chat command: no free account on this key"
|
||||
|
||||
Reference in New Issue
Block a user