Merge pull request #525 from LedgerHQ/fix/apa/chain_id_check_for_clones

Improve error-handling of chain ID when parsing APDUs

src/network.c

@@ -144,3 +144,15 @@ const char *get_displayable_ticker(const uint64_t *chain_id) {
     }
     return ticker;
 }
+
+/**
+ * Checks wether the app can support the given chain ID
+ *
+ * - If the given chain ID is the same as the app's one
+ * - If both chain IDs are present in the array of Ethereum-compatible networks
+ */
+bool app_compatible_with_chain_id(const uint64_t *chain_id) {
+    return ((chainConfig->chainId == *chain_id) ||
+            (chain_is_ethereum_compatible(&chainConfig->chainId) &&
+             chain_is_ethereum_compatible(chain_id)));
+}

src/network.h

@@ -3,10 +3,16 @@
 #include <stdint.h>
 #include <stdbool.h>
 
+#define UNSUPPORTED_CHAIN_ID_MSG(id)                                              \
+    do {                                                                          \
+        PRINTF("Unsupported chain ID: %u (app: %u)\n", id, chainConfig->chainId); \
+    } while (0)
+
 const char *get_network_name_from_chain_id(const uint64_t *chain_id);
 const char *get_network_ticker_from_chain_id(const uint64_t *chain_id);
 
 bool chain_is_ethereum_compatible(const uint64_t *chain_id);
+bool app_compatible_with_chain_id(const uint64_t *chain_id);
 
 uint64_t get_tx_chain_id(void);
 

src_features/provideErc20TokenInformation/cmd_provideTokenInfo.c

@@ -3,6 +3,7 @@
 #include "public_keys.h"
 #include "common_ui.h"
 #include "os_io_seproxyhal.h"
+#include "network.h"
 
 #ifdef HAVE_CONTRACT_NAME_IN_DESCRIPTOR
 
@@ -111,7 +112,7 @@ void handleProvideErc20TokenInformation(uint8_t p1,
     UNUSED(tx);
     uint32_t offset = 0;
     uint8_t tickerLength;
-    uint32_t chainId;
+    uint64_t chain_id;
     uint8_t hash[INT256_LENGTH];
     cx_ecfp_public_key_t tokenKey;
 
@@ -141,12 +142,13 @@ void handleProvideErc20TokenInformation(uint8_t p1,
     memmove(token->address, workBuffer + offset, 20);
     offset += 20;
     dataLength -= 20;
+    // TODO: Handle 64-bit long chain IDs
     token->decimals = U4BE(workBuffer, offset);
     offset += 4;
     dataLength -= 4;
-    chainId = U4BE(workBuffer, offset);
-    if ((chainConfig->chainId != ETHEREUM_MAINNET_CHAINID) && (chainConfig->chainId != chainId)) {
-        PRINTF("ChainId token mismatch: %d vs %d\n", chainConfig->chainId, chainId);
+    chain_id = U4BE(workBuffer, offset);
+    if (!app_compatible_with_chain_id(&chain_id)) {
+        UNSUPPORTED_CHAIN_ID_MSG(chain_id);
         THROW(0x6A80);
     }
     offset += 4;

src_features/provideNFTInformation/cmd_provideNFTInfo.c

@@ -130,8 +130,8 @@ void handleProvideNFTInformation(uint8_t p1,
     // this prints raw data, so to have a more meaningful print, display
     // the buffer before the endianness swap
     PRINTF("ChainID: %.*H\n", sizeof(chain_id), (workBuffer + offset));
-    if (!chain_is_ethereum_compatible(&chain_id)) {
-        PRINTF("Unsupported chain ID!\n");
+    if (!app_compatible_with_chain_id(&chain_id)) {
+        UNSUPPORTED_CHAIN_ID_MSG(chain_id);
         THROW(APDU_RESPONSE_INVALID_DATA);
     }
     offset += CHAIN_ID_SIZE;

src_features/setPlugin/cmd_setPlugin.c

@@ -159,8 +159,8 @@ void handleSetPlugin(uint8_t p1,
     // this prints raw data, so to have a more meaningful print, display
     // the buffer before the endianness swap
     PRINTF("ChainID: %.*H\n", sizeof(chain_id), (workBuffer + offset));
-    if (!chain_is_ethereum_compatible(&chain_id)) {
-        PRINTF("Unsupported chain ID!\n");
+    if (!app_compatible_with_chain_id(&chain_id)) {
+        UNSUPPORTED_CHAIN_ID_MSG(chain_id);
         THROW(APDU_RESPONSE_INVALID_DATA);
     }
     offset += CHAIN_ID_SIZE;