legacy, tests: Check for invalid PIN.

legacy/firmware/pinmatrix.c

@@ -63,18 +63,21 @@ void pinmatrix_start(const char *text) {
   pinmatrix_draw(text);
 }
 
-void pinmatrix_done(char *pin) {
+secbool pinmatrix_done(char *pin) {
   int i = 0, k = 0;
+  secbool ret = sectrue;
   while (pin && pin[i]) {
     k = pin[i] - '1';
     if (k >= 0 && k <= 8) {
       pin[i] = pinmatrix_perm[k];
     } else {
       pin[i] = 'X';
+      ret = secfalse;
     }
     i++;
   }
   memset(pinmatrix_perm, 'X', sizeof(pinmatrix_perm) - 1);
+  return ret;
 }
 
 #if DEBUG_LINK

legacy/firmware/pinmatrix.h

@@ -20,8 +20,10 @@
 #ifndef __PINMATRIX_H__
 #define __PINMATRIX_H__
 
+#include "secbool.h"
+
 void pinmatrix_start(const char *text);
-void pinmatrix_done(char *pin);
+secbool pinmatrix_done(char *pin);
 const char *pinmatrix_get(void);
 
 #endif

legacy/firmware/protect.c

@@ -124,9 +124,11 @@ const char *requestPin(PinMatrixRequestType type, const char *text) {
     if (msg_tiny_id == MessageType_MessageType_PinMatrixAck) {
       msg_tiny_id = 0xFFFF;
       PinMatrixAck *pma = (PinMatrixAck *)msg_tiny;
-      pinmatrix_done(pma->pin);  // convert via pinmatrix
       usbTiny(0);
-      return pma->pin;
+      if (sectrue == pinmatrix_done(pma->pin))  // convert via pinmatrix
+        return pma->pin;
+      else
+        return 0;
     }
     // check for Cancel / Initialize
     protectAbortedByCancel = (msg_tiny_id == MessageType_MessageType_Cancel);
@@ -243,7 +245,7 @@ bool protectChangePin(bool removal) {
   if (!removal) {
     pin = requestPin(PinMatrixRequestType_PinMatrixRequestType_NewFirst,
                      _("Please enter new PIN:"));
-    if (pin == NULL) {
+    if (pin == NULL || pin[0] == '\0') {
       memzero(old_pin, sizeof(old_pin));
       fsm_sendFailure(FailureType_Failure_PinCancelled, NULL);
       return false;

tests/device_tests/test_msg_change_wipe_code_t1.py

@@ -174,3 +174,26 @@ def test_set_pin_to_wipe_code(client):
     assert client.features.pin_protection is False
     resp = client.call_raw(messages.GetAddress())
     assert isinstance(resp, messages.Address)
+
+
+@pytest.mark.parametrize("invalid_wipe_code", ("1204", "", "1234567891"))
+def test_set_wipe_code_invalid(client, invalid_wipe_code):
+    # Let's set the wipe code
+    ret = client.call_raw(messages.ChangeWipeCode())
+    assert isinstance(ret, messages.ButtonRequest)
+
+    # Confirm
+    client.debug.press_yes()
+    ret = client.call_raw(messages.ButtonAck())
+
+    # Enter a wipe code containing an invalid digit
+    assert isinstance(ret, messages.PinMatrixRequest)
+    assert ret.type == PinType.WipeCodeFirst
+    ret = client.call_raw(messages.PinMatrixAck(pin=invalid_wipe_code))
+
+    # Ensure the invalid wipe code is detected
+    assert isinstance(ret, messages.Failure)
+
+    # Check that there's still no wipe code protection.
+    client.init_device()
+    assert client.features.wipe_code_protection is False

tests/device_tests/test_msg_changepin.py

@@ -138,7 +138,7 @@ class TestMsgChangepin:
         ret = client.call_raw(proto.GetAddress())
         assert isinstance(ret, proto.Address)
 
-    def test_set_failed(self, client):
+    def test_set_mismatch(self, client):
         features = client.call_raw(proto.Initialize())
         assert features.pin_protection is False
 
@@ -174,7 +174,7 @@ class TestMsgChangepin:
         assert isinstance(ret, proto.Address)
 
     @pytest.mark.setup_client(pin=True)
-    def test_set_failed_2(self, client):
+    def test_change_mismatch(self, client):
         features = client.call_raw(proto.Initialize())
         assert features.pin_protection is True
 
@@ -209,6 +209,58 @@ class TestMsgChangepin:
         assert features.pin_protection is True
         self.check_pin(client, PIN4)
 
+    @pytest.mark.parametrize("invalid_pin", ("1204", "", "1234567891"))
+    def test_set_invalid(self, client, invalid_pin):
+        features = client.call_raw(proto.Initialize())
+        assert features.pin_protection is False
+
+        # Let's set an invalid PIN
+        ret = client.call_raw(proto.ChangePin())
+        assert isinstance(ret, proto.ButtonRequest)
+
+        # Press button
+        client.debug.press_yes()
+        ret = client.call_raw(proto.ButtonAck())
+
+        # Send a PIN containing an invalid digit
+        assert isinstance(ret, proto.PinMatrixRequest)
+        ret = client.call_raw(proto.PinMatrixAck(pin=invalid_pin))
+
+        # Ensure the invalid PIN is detected
+        assert isinstance(ret, proto.Failure)
+
+        # Check that there's still no PIN protection now
+        features = client.call_raw(proto.Initialize())
+        assert features.pin_protection is False
+        ret = client.call_raw(proto.GetAddress())
+        assert isinstance(ret, proto.Address)
+
+    @pytest.mark.parametrize("invalid_pin", ("1204", "", "1234567891"))
+    @pytest.mark.setup_client(pin=True)
+    def test_remove_invalid(self, client, invalid_pin):
+        features = client.call_raw(proto.Initialize())
+        assert features.pin_protection is True
+
+        # Let's change the PIN
+        ret = client.call_raw(proto.ChangePin(remove=True))
+        assert isinstance(ret, proto.ButtonRequest)
+
+        # Press button
+        client.debug.press_yes()
+        ret = client.call_raw(proto.ButtonAck())
+
+        # Instead of the old PIN, send a PIN containing an invalid digit
+        assert isinstance(ret, proto.PinMatrixRequest)
+        ret = client.call_raw(proto.PinMatrixAck(pin=invalid_pin))
+
+        # Ensure the invalid PIN is detected
+        assert isinstance(ret, proto.Failure)
+
+        # Check that there's still old PIN protection
+        features = client.call_raw(proto.Initialize())
+        assert features.pin_protection is True
+        self.check_pin(client, PIN4)
+
     def check_pin(self, client, pin):
         client.clear_session()
         ret = client.call_raw(proto.GetAddress())