storage/tests/tests/storage_model.py (1)

40-48: Inconsistent behavior: unlock() vs check_pin() for old PIN verification.

The StorageModel.change_pin() uses self.unlock(oldpin) to verify the old PIN (line 43), but the equivalent implementation in storage/tests/python/src/storage.py uses self.check_pin(oldpin) instead. This creates a behavioral difference:

• unlock() decrements pin_rem and can trigger a wipe on wrong PIN
• check_pin() (if it exists in this model) would typically just verify without side effects

Since the storage is already unlocked at this point, calling unlock() with a wrong oldpin would unexpectedly decrement the PIN counter and potentially wipe storage, which seems like unintended behavior for a PIN change operation.

Consider aligning with the Python storage implementation by using a check_pin() method instead, or documenting this as intentional behavior.

core/src/apps/management/change_pin.py (1)

41-54: Replace config.check_pin() with config.unlock() to unlock storage before calling change_pin().

The config.change_pin() call requires unlocked storage (per mock docstring), but the code only calls config.check_pin(curpin, salt), which validates the PIN without unlocking. This should be config.unlock(curpin, salt) instead.

core/embed/sys/smcall/stm32/smcall_verifiers.c (2)

278-281: Critical: Type mismatch returns success on access violation.

The function returns storage_unlock_result_t, but the access_violation path returns secfalse. If secfalse equals 0 (which is common), this would incorrectly return UNLOCK_OK (which is also 0), indicating a successful unlock on access violation.

This is a security issue - an access violation should return a failure code like UNLOCK_UNKNOWN or a dedicated access violation error code.

🐛 Proposed fix

 access_violation:
   apptask_access_violation();
-  return secfalse;
+  return UNLOCK_UNKNOWN;
 }

---

295-298: Critical: Type mismatch returns success on access violation.

Same issue as storage_unlock__verified - returning secfalse for a storage_pin_change_result_t would incorrectly indicate PIN_CHANGE_OK if secfalse equals 0.

🐛 Proposed fix

 access_violation:
   apptask_access_violation();
-  return secfalse;
+  return PIN_CHANGE_UNKNOWN;
 }

core/embed/sys/syscall/stm32/syscall_verifiers.c (2)

655-658: Critical: Type mismatch returns success on access violation.

Same issue as in smcall_verifiers.c - the function returns storage_unlock_result_t, but secfalse is returned on access violation, which would incorrectly indicate UNLOCK_OK if secfalse equals 0.

🐛 Proposed fix

 access_violation:
   apptask_access_violation();
-  return secfalse;
+  return UNLOCK_UNKNOWN;
 }

---

672-675: Critical: Type mismatch returns success on access violation.

Same issue - returning secfalse for a storage_pin_change_result_t would incorrectly indicate PIN_CHANGE_OK.

🐛 Proposed fix

 access_violation:
   apptask_access_violation();
-  return secfalse;
+  return PIN_CHANGE_UNKNOWN;
 }