Add an explicit mechanism for the Monero app to refuse some client ve…

…rsions
LedgerHQ · Jul 25, 2022 · a4a03b0 · a4a03b0
1 parent 1c7bb4b
commit a4a03b0
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 32 deletions.
diff --git a/src/monero_init.c b/src/monero_init.c
@@ -177,33 +177,44 @@ void monero_install(unsigned char netId) {
 /* ----------------------------------------------------------------------- */
 /* --- Reset                                                           --- */
 /* ----------------------------------------------------------------------- */
+// Accept the following versions and their derivates
 const char* const monero_supported_client[] = {"0.18."};
 #define MONERO_SUPPORTED_CLIENT_SIZE \
     (sizeof(monero_supported_client) / sizeof(monero_supported_client[0]))
 
+// Explicitly refuse the following versions
+const char* const monero_refused_client[] = {"0.18.0.0."};
+#define MONERO_REFUSED_CLIENT_SIZE \
+    (sizeof(monero_refused_client) / sizeof(monero_refused_client[0]))
+
 int monero_apdu_reset() {
     unsigned int client_version_len;
     char client_version[16];
+    memset(client_version, '\0', 16);
     client_version_len = G_monero_vstate.io_length - G_monero_vstate.io_offset;
     if (client_version_len > 14) {
         THROW(SW_CLIENT_NOT_SUPPORTED + 1);
     }
     monero_io_fetch((unsigned char*)&client_version[0], client_version_len);
+    // Add '.' suffix to avoid 'X.1' prefixing 'X.10'
     client_version[client_version_len] = '.';
-    client_version_len++;
-    client_version[client_version_len] = 0;
-    unsigned int i = 0;
-    while (i < MONERO_SUPPORTED_CLIENT_SIZE) {
+    // Check if version is explicitly refused
+    for (uint32_t i = 0; i < MONERO_REFUSED_CLIENT_SIZE; ++i) {
+        if (strcmp(PIC(monero_refused_client[i]), client_version) == 0) {
+            THROW(SW_CLIENT_NOT_SUPPORTED);
+        }
+    }
+    // Check if version is supported
+    uint32_t i;
+    for (i = 0; i < MONERO_SUPPORTED_CLIENT_SIZE + 1; ++i) {
+        // Use strncmp to allow supported version prefixing client version
         unsigned int monero_supported_client_len = strlen((char*)PIC(monero_supported_client[i]));
-        if ((monero_supported_client_len <= client_version_len) &&
-            (os_memcmp((char*)PIC(monero_supported_client[i]), client_version,
-                       monero_supported_client_len) == 0)) {
+        if (strncmp(PIC(monero_supported_client[i]), client_version, monero_supported_client_len) == 0) {
             break;
         }
-        i++;
-    }
-    if (i == MONERO_SUPPORTED_CLIENT_SIZE) {
-        THROW(SW_CLIENT_NOT_SUPPORTED);
+        if (i == MONERO_SUPPORTED_CLIENT_SIZE) {
+            THROW(SW_CLIENT_NOT_SUPPORTED);
+        }
     }
 
     monero_io_discard(0);

diff --git a/tests/test_sig.py b/tests/test_sig.py
@@ -48,12 +48,9 @@ def state():
 
     @staticmethod
     def test_set_sig(monero):
-        # Send a reset, ignore the version check and version related errors.
-        # They belong in a dedicated test
-        with pytest.raises(ClientNotSupported) as excinfo:
-            monero.reset_and_get_version(
-                monero_client_version=b"0.0.0.0"
-            )
+        monero.reset_and_get_version(
+            monero_client_version=b"0.18"
+        )
 
         sig_mode: SigType = monero.set_signature_mode(sig_type=SigType.REAL)
         assert sig_mode == SigType.REAL

diff --git a/tests/test_version.py b/tests/test_version.py
@@ -3,25 +3,38 @@
 # pylint: disable=wildcard-import, unused-wildcard-import
 from monero_client.exception import ClientNotSupported
 
-
-def test_version(monero):
-    # Monero does not have any consensus changes with minor point upgrades, we support 0.18.
-
+def check_accepted_version(monero, valid_version: bytes):
     major, minor, patch = monero.reset_and_get_version(
-        monero_client_version=b"0.18.0.0"
+        monero_client_version=valid_version
     )  # type: int, int, int
+    assert (major, minor, patch) == (1, 8, 0)  # version of the Monero app
 
-    assert (major, minor) == (1, 8)  # version of the Monero app
+def check_refused_version(monero, invalid_version: bytes):
+    with pytest.raises(ClientNotSupported) as excinfo:
+        monero.reset_and_get_version(invalid_version)
 
-    major, minor, patch = monero.reset_and_get_version(
-        monero_client_version=b"0.18.9.0"
-    )  # type: int, int, int
 
-    assert (major, minor) == (1, 8)  # version of the Monero app
+def test_version(monero):
+    # Monero does not have any consensus changes with minor point upgrades, we support the entire 0.18. range, except 0.18.0.0
+    check_accepted_version(monero, b"0.18")
+    check_accepted_version(monero, b"0.18.0.1")
+    check_accepted_version(monero, b"0.18.0.10")
+    check_accepted_version(monero, b"0.18.1.1")
+    check_accepted_version(monero, b"0.18.9.0")
+    check_accepted_version(monero, b"0.18.18.0")
 
 def test_old_client_version(monero):
-    # should raise ClientNotSupported[0x6a30]
-    with pytest.raises(ClientNotSupported) as excinfo:
-        monero.reset_and_get_version(b"0.17.0.0")
-    with pytest.raises(ClientNotSupported) as excinfo:
-        monero.reset_and_get_version(b"0.19.0.0")
+    # Not supported anymore
+    check_refused_version(monero, b"0.0.0.0")
+    check_refused_version(monero, b"0.17.0.0")
+    check_refused_version(monero, b"0.17.18.0")
+    # Explicitly disabled
+    check_refused_version(monero, b"0.18.0.0")
+    # Not yet supported
+    check_refused_version(monero, b"0.19.0.0")
+    check_refused_version(monero, b"1.0.0.0")
+    # Regex shenanigans
+    check_refused_version(monero, b"")
+    check_refused_version(monero, b"0")
+    check_refused_version(monero, b"0.0.")
+    check_refused_version(monero, b"0.180")