diff --git a/legacy/firmware/pinmatrix.c b/legacy/firmware/pinmatrix.c index 094e4c995..9d415cf3d 100644 --- a/legacy/firmware/pinmatrix.c +++ b/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 diff --git a/legacy/firmware/pinmatrix.h b/legacy/firmware/pinmatrix.h index 5370b98d5..4ebbba9bf 100644 --- a/legacy/firmware/pinmatrix.h +++ b/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 diff --git a/legacy/firmware/protect.c b/legacy/firmware/protect.c index 5169e3324..28dda1fad 100644 --- a/legacy/firmware/protect.c +++ b/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; diff --git a/tests/device_tests/test_msg_change_wipe_code_t1.py b/tests/device_tests/test_msg_change_wipe_code_t1.py index 97f3c9744..df2a57b3f 100644 --- a/tests/device_tests/test_msg_change_wipe_code_t1.py +++ b/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 diff --git a/tests/device_tests/test_msg_changepin.py b/tests/device_tests/test_msg_changepin.py index 62aacad92..6b386b5f9 100644 --- a/tests/device_tests/test_msg_changepin.py +++ b/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())