fix(simulator): return password error on PIN mismatch in SimulatorLoadAccountSecret
What the developer wrote
Authored by soralit
fix(simulator): return password error on PIN mismatch in SimulatorLoadAccountSecret
SimulatorLoadAccountSecret returned SUCCESS even when the stored password
did not match. This made the duplicate-PIN check (CheckPasswordExisted ->
VerifyAccountPassword) always treat any new PIN as a duplicate of an
existing account, so setting a PIN for the second account and onwards
failed with 'Duplicate PIN code detected'.
Return ERR_KEYSTORE_PASSWORD_ERR on mismatch, matching the real-device
behavior (LoadAccountSecretFromSE returns ERR_KEYSTORE_AUTH on HMAC
failure, which VerifyAccountPassword maps to ERR_KEYSTORE_PASSWORD_ERR).
What changed, and why it matters
This commit fixes a bug in the firmware's simulator (a software stand-in used for testing, not the real hardware wallet). When checking a stored account password/PIN, the simulator was accidentally returning 'success' even when the password did not match. That caused the wallet to wrongly think every new PIN was already in use, blocking users from creating a second or later account in simulator builds. The fix makes the simulator return a password-error code on mismatch, matching how the real device behaves. It is a functional bug fix in test-only code, not a security vulnerability in the real wallet.
No security response required. Treat as a normal functional bug fix in simulator/test code. Verify that simulator integration tests for multi-account creation now pass and that real-device behavior is unchanged.
Security signals we found
Incorrect success return on authentication failure in simulator-only code
Mismatch between simulator and real-device authentication error semantics
Functional regression in multi-account PIN creation in simulator builds
Evidence from the diff
In ui_simulator/simulator_storage.c, SimulatorLoadAccountSecret previously returned the local variable ret (initialized to SUCCESS/0) when the JSON-stored password did not match the supplied password. This incorrect success return propagated through VerifyAccountPassword so CheckPasswordExisted flagged every candidate PIN as already existing, preventing multi-account PIN setup in simulator builds. The patch changes the mismatch path to return ERR_KEYSTORE_PASSWORD_ERR, aligning with the real-device path (LoadAccountSecretFromSE -> ERR_KEYSTORE_AUTH -> VerifyAccountPassword maps to ERR_KEYSTORE_PASSWORD_ERR). The change is confined to simulator code and does not alter real-device secure-element behavior.
Changed components
ui_simulator/simulator_storage.cSimulatorLoadAccountSecretCheckPasswordExisted / VerifyAccountPassword (callers, not modified)Inspect captured patch +4 / −1
diff --git a/ui_simulator/simulator_storage.c b/ui_simulator/simulator_storage.c
index f95422e..2cdcb18 100644
--- a/ui_simulator/simulator_storage.c
+++ b/ui_simulator/simulator_storage.c
@@ -295,7 +295,10 @@ int32_t SimulatorLoadAccountSecret(uint8_t accountIndex, AccountSecret_t *accoun
cJSON *passwordJson = cJSON_GetObjectItem(rootJson, "password");
if (passwordJson == NULL || strcmp(passwordJson->valuestring, password) != 0) {
cJSON_Delete(rootJson);
- return ret;
+ // Password mismatch must NOT be reported as success, otherwise the
+ // duplicate-PIN check (CheckPasswordExisted -> VerifyAccountPassword)
+ // always treats any new PIN as a duplicate of an existing account.
+ return ERR_KEYSTORE_PASSWORD_ERR;
}
GetJsonArrayData(rootJson, accountSecret->entropy, ENTROPY_MAX_LEN, "entropy");
GetJsonArrayData(rootJson, accountSecret->seed, SEED_LEN, "seed");
Why this scored 19/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.