feat(legacy): Strict path validation for Ethereum.

legacy/firmware/.changelog.d/noissue.security

@@ -0,0 +1 @@
+Strict path validations for altcoins.

legacy/firmware/ethereum.c

@@ -1040,3 +1040,52 @@ bool ethereum_parse(const char *address, uint8_t pubkeyhash[20]) {
   }
   return true;
 }
+
+bool ethereum_path_check(uint32_t address_n_count, const uint32_t *address_n,
+                         bool pubkey_export, uint64_t chain) {
+  bool valid = (address_n_count >= 3);
+  valid = valid && (address_n[0] == (PATH_HARDENED | 44));
+  valid = valid && (address_n[1] & PATH_HARDENED);
+  valid = valid && (address_n[2] & PATH_HARDENED);
+  valid = valid && ((address_n[2] & PATH_UNHARDEN_MASK) <= PATH_MAX_ACCOUNT);
+
+  uint32_t path_slip44 = address_n[1] & PATH_UNHARDEN_MASK;
+  if (chain == CHAIN_ID_UNKNOWN) {
+    valid = valid && (is_ethereum_slip44(path_slip44));
+  } else {
+    uint32_t chain_slip44 = ethereum_slip44_by_chain_id(chain);
+    if (chain_slip44 == SLIP44_UNKNOWN) {
+      // Allow Ethereum or testnet paths for unknown networks.
+      valid = valid && (path_slip44 == 60 || path_slip44 == 1);
+    } else if (chain_slip44 != 60 && chain_slip44 != 1) {
+      // Allow cross-signing with Ethereum unless it's testnet.
+      valid = valid && (path_slip44 == chain_slip44 || path_slip44 == 60);
+    } else {
+      valid = valid && (path_slip44 == chain_slip44);
+    }
+  }
+
+  if (pubkey_export) {
+    // m/44'/coin_type'/account'/*
+    return valid;
+  }
+
+  if (address_n_count == 3) {
+    // SEP-0005 for non-UTXO-based currencies, defined by Stellar:
+    // https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0005.md
+    // m/44'/coin_type'/account'
+    return valid;
+  }
+
+  // We believe Ethereum should use the SEP-0005 scheme for everything, because
+  // it is account-based, rather than UTXO-based. Unfortunately, a lot of
+  // Ethereum tools (MEW, Metamask) do not use such scheme and set account = 0
+  // and then iterate the address index. For compatibility, we allow this scheme
+  // as well.
+  // m/44'/coin_type'/account'/change/address_index
+  valid = valid && (address_n_count == 5);
+  valid = valid && (address_n[3] <= PATH_MAX_CHANGE);
+  valid = valid && (address_n[4] <= PATH_MAX_ADDRESS_INDEX);
+
+  return valid;
+}

legacy/firmware/ethereum.h

@@ -25,6 +25,8 @@
 #include "bip32.h"
 #include "messages-ethereum.pb.h"
 
+#define CHAIN_ID_UNKNOWN UINT64_MAX
+
 void ethereum_signing_init(const EthereumSignTx *msg, const HDNode *node);
 void ethereum_signing_init_eip1559(const EthereumSignTxEIP1559 *msg,
                                    const HDNode *node);
@@ -39,4 +41,6 @@ void ethereum_typed_hash_sign(const EthereumSignTypedHash *msg,
                               EthereumTypedDataSignature *resp);
 bool ethereum_parse(const char *address, uint8_t pubkeyhash[20]);
 
+bool ethereum_path_check(uint32_t address_n_count, const uint32_t *address_n,
+                         bool pubkey_export, uint64_t chain);
 #endif

legacy/firmware/ethereum_networks.h.mako

@@ -3,14 +3,19 @@ BKSL = "\\"
 
 networks = list(supported_on("trezor1", eth))
 max_chain_id_length = 0
+max_slip44_length = 0
 max_suffix_length = 0
 for n in networks:
 	max_chain_id_length = max(len(str(n.chain_id)), max_chain_id_length)
+	max_slip44_length = max(len(str(n.slip44)), max_slip44_length)
 	max_suffix_length = max(len(n.shortcut), max_suffix_length)
 
 def align_chain_id(n):
 	return "{:>{w}}".format(n.chain_id, w=max_chain_id_length)
 
+def align_slip44(n):
+	return "{:>{w}}".format(n.slip44, w=max_slip44_length)
+
 def align_suffix(n):
 	cstr = c_str(" " + n.shortcut) + ";"
 	# we add two quotes, a space and a semicolon. hence +4 chars
@@ -23,12 +28,34 @@ def align_suffix(n):
 #ifndef __ETHEREUM_NETWORKS_H__
 #define __ETHEREUM_NETWORKS_H__
 
+#define SLIP44_UNKNOWN UINT32_MAX
+
 #define ASSIGN_ETHEREUM_SUFFIX(suffix, chain_id) ${BKSL}
-			switch (chain_id) { ${BKSL}
+  switch (chain_id) { ${BKSL}
 % for n in networks:
-				case ${align_chain_id(n)}: suffix = ${align_suffix(n)} break;  /* ${n.name} */ ${BKSL}
+    case ${align_chain_id(n)}: suffix = ${align_suffix(n)} break;  /* ${n.name} */ ${BKSL}
 % endfor
-				default: suffix = " UNKN"; break;  /* unknown chain */ ${BKSL}
+    default: suffix = " UNKN"; break;  /* unknown chain */ ${BKSL}
-			}
+  }
+
+static bool is_ethereum_slip44(uint32_t slip44) {
+  switch (slip44) {
+% for slip44 in sorted(set(n.slip44 for n in networks)):
+    case ${slip44}:
+% endfor
+      return true;
+    default:
+      return false;
+  }
+}
+
+static int32_t ethereum_slip44_by_chain_id(uint64_t chain_id) {
+  switch (chain_id) {
+% for n in networks:
+    case ${align_chain_id(n)}: return ${align_slip44(n)};  /* ${n.name} */
+% endfor
+    default: return SLIP44_UNKNOWN;  /* unknown chain */
+  }
+}
 
 #endif

legacy/firmware/fsm_msg_ethereum.h

@@ -17,6 +17,22 @@
  * along with this library.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+static bool fsm_ethereumCheckPath(uint32_t address_n_count,
+                                  const uint32_t *address_n, bool pubkey_export,
+                                  uint64_t chain_id) {
+  if (ethereum_path_check(address_n_count, address_n, pubkey_export,
+                          chain_id)) {
+    return true;
+  }
+
+  if (config_getSafetyCheckLevel() == SafetyCheckLevel_Strict) {
+    fsm_sendFailure(FailureType_Failure_DataError, _("Forbidden key path"));
+    return false;
+  }
+
+  return fsm_layoutPathWarning();
+}
+
 void fsm_msgEthereumGetPublicKey(const EthereumGetPublicKey *msg) {
   RESP_INIT(EthereumPublicKey);
 
@@ -28,6 +44,12 @@ void fsm_msgEthereumGetPublicKey(const EthereumGetPublicKey *msg) {
   const CoinInfo *coin = fsm_getCoin(true, "Bitcoin");
   if (!coin) return;
 
+  if (!fsm_ethereumCheckPath(msg->address_n_count, msg->address_n, true,
+                             CHAIN_ID_UNKNOWN)) {
+    layoutHome();
+    return;
+  }
+
   const char *curve = coin->curve_name;
   uint32_t fingerprint;
   HDNode *node = fsm_getDerivedNode(curve, msg->address_n, msg->address_n_count,
@@ -71,6 +93,12 @@ void fsm_msgEthereumSignTx(const EthereumSignTx *msg) {
 
   CHECK_PIN
 
+  if (!fsm_ethereumCheckPath(msg->address_n_count, msg->address_n, false,
+                             msg->chain_id)) {
+    layoutHome();
+    return;
+  }
+
   const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n,
                                           msg->address_n_count, NULL);
   if (!node) return;
@@ -83,6 +111,12 @@ void fsm_msgEthereumSignTxEIP1559(const EthereumSignTxEIP1559 *msg) {
 
   CHECK_PIN
 
+  if (!fsm_ethereumCheckPath(msg->address_n_count, msg->address_n, false,
+                             msg->chain_id)) {
+    layoutHome();
+    return;
+  }
+
   const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n,
                                           msg->address_n_count, NULL);
   if (!node) return;
@@ -101,13 +135,22 @@ void fsm_msgEthereumGetAddress(const EthereumGetAddress *msg) {
 
   CHECK_PIN
 
+  if (!fsm_ethereumCheckPath(msg->address_n_count, msg->address_n, false,
+                             CHAIN_ID_UNKNOWN)) {
+    layoutHome();
+    return;
+  }
+
   const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n,
                                           msg->address_n_count, NULL);
   if (!node) return;
 
   uint8_t pubkeyhash[20];
 
-  if (!hdnode_get_ethereum_pubkeyhash(node, pubkeyhash)) return;
+  if (!hdnode_get_ethereum_pubkeyhash(node, pubkeyhash)) {
+    layoutHome();
+    return;
+  }
 
   uint32_t slip44 =
       (msg->address_n_count > 1) ? (msg->address_n[1] & PATH_UNHARDEN_MASK) : 0;
@@ -150,12 +193,19 @@ void fsm_msgEthereumSignMessage(const EthereumSignMessage *msg) {
 
   CHECK_PIN
 
+  if (!fsm_ethereumCheckPath(msg->address_n_count, msg->address_n, false,
+                             CHAIN_ID_UNKNOWN)) {
+    layoutHome();
+    return;
+  }
+
   const HDNode *node = fsm_getDerivedNode(SECP256K1_NAME, msg->address_n,
                                           msg->address_n_count, NULL);
   if (!node) return;
 
   uint8_t pubkeyhash[20] = {0};
   if (!hdnode_get_ethereum_pubkeyhash(node, pubkeyhash)) {
+    layoutHome();
     return;
   }
 
@@ -230,6 +280,12 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash *msg) {
     return;
   }
 
+  if (!fsm_ethereumCheckPath(msg->address_n_count, msg->address_n, false,
+                             CHAIN_ID_UNKNOWN)) {
+    layoutHome();
+    return;
+  }
+
   layoutDialogSwipe(&bmp_icon_warning, _("Abort"), _("Continue"), NULL,
                     _("Unable to show"), _("EIP-712 data."), NULL,
                     _("Sign at your own risk."), NULL, NULL);
@@ -245,6 +301,7 @@ void fsm_msgEthereumSignTypedHash(const EthereumSignTypedHash *msg) {
 
   uint8_t pubkeyhash[20] = {0};
   if (!hdnode_get_ethereum_pubkeyhash(node, pubkeyhash)) {
+    layoutHome();
     return;
   }