Merge pull request #559 from LedgerHQ/parse_unhardened

### src/common/wallet.c
@@ -560,7 +560,7 @@ static int parse_keyexpr(buffer_t *in_buf,
         } else if (next_character == '<') {
             buffer_seek_cur(in_buf, 1);  // skip "<"
             if (parse_unsigned_decimal(in_buf, &out->num_first) == -1 ||
-                out->num_first > 0x80000000u) {
+                out->num_first >= BIP32_FIRST_HARDENED_CHILD) {
                 return WITH_ERROR(
                     -1,
                     "Expected /** or /<M;N>/* in key expression, with unhardened M and N");
@@ -571,7 +571,7 @@ static int parse_keyexpr(buffer_t *in_buf,
             }
 
             if (parse_unsigned_decimal(in_buf, &out->num_second) == -1 ||
-                out->num_second > 0x80000000u) {
+                out->num_second >= BIP32_FIRST_HARDENED_CHILD) {
                 return WITH_ERROR(
                     -1,
                     "Expected /** or /<M;N>/* in key expression, with unhardened M and N");

### src/constants.h
@@ -33,22 +33,22 @@
 /**
  * Maximum DER-encoded signature length (bytes).
  */
-#define MAX_DER_SIG_LEN 72
+#define MAX_DER_SIG_LEN 72U
 
 /**
  * Maximum scriptPubKey length for an input that we can sign.
  */
-#define MAX_PREVOUT_SCRIPTPUBKEY_LEN 34  // P2WSH's scriptPubKeys are the longest supported
+#define MAX_PREVOUT_SCRIPTPUBKEY_LEN 34U  // P2WSH's scriptPubKeys are the longest supported
 
 /**
  * Maximum scriptPubKey length for an output that we can recognize.
  */
-#define MAX_OUTPUT_SCRIPTPUBKEY_LEN 83  // max 83 for OP_RETURN; other scripts are shorter
+#define MAX_OUTPUT_SCRIPTPUBKEY_LEN 83U  // max 83 for OP_RETURN; other scripts are shorter
 
 /**
  * Maximum length of a wallet registered into the device (characters), excluding terminating NULL.
  */
-#define MAX_WALLET_NAME_LENGTH 64
+#define MAX_WALLET_NAME_LENGTH 64U
 
 /**
  * Maximum length of output index string
@@ -64,42 +64,42 @@
 #error "bolos_target.h must be included (TARGET_* constants unavailable)"
 #endif
 #ifdef TARGET_NANOX
-#define MAX_EXT_OUTPUT_SIMPLIFIED_NUMBER 8
+#define MAX_EXT_OUTPUT_SIMPLIFIED_NUMBER 8U
 #else
-#define MAX_EXT_OUTPUT_SIMPLIFIED_NUMBER 16
+#define MAX_EXT_OUTPUT_SIMPLIFIED_NUMBER 16U
 #endif
 
 /**
  * Maximum length (characters) of a base58check-encoded serialized extended pubkey.
  */
-#define MAX_SERIALIZED_PUBKEY_LENGTH 113
+#define MAX_SERIALIZED_PUBKEY_LENGTH 113U
 
 /**
  * Maximum number of inputs supported while signing a transaction.
  */
-#define MAX_N_INPUTS_CAN_SIGN 512
+#define MAX_N_INPUTS_CAN_SIGN 512U
 
 /**
  * Maximum number of outputs supported while signing a transaction.
  */
-#define MAX_N_OUTPUTS_CAN_SIGN 512
+#define MAX_N_OUTPUTS_CAN_SIGN 512U
 
 /**
  * Maximum supported number of internal key expressions in a wallet policy.
  * A key expression is internal if we can sign for it (either as an individual key,
  * or as part of a MuSig key expression).
  */
-#define MAX_INTERNAL_KEY_EXPRESSIONS 8
+#define MAX_INTERNAL_KEY_EXPRESSIONS 8U
 
 // SIGHASH flags
-#define SIGHASH_DEFAULT      0x00000000
-#define SIGHASH_ALL          0x00000001
-#define SIGHASH_NONE         0x00000002
-#define SIGHASH_SINGLE       0x00000003
-#define SIGHASH_ANYONECANPAY 0x00000080
+#define SIGHASH_DEFAULT      0x00000000U
+#define SIGHASH_ALL          0x00000001U
+#define SIGHASH_NONE         0x00000002U
+#define SIGHASH_SINGLE       0x00000003U
+#define SIGHASH_ANYONECANPAY 0x00000080U
 
-#define SEQUENCE_LOCKTIME_TYPE_FLAG (1 << 22)
-#define LOCKTIME_THRESHOLD          500000000
+#define SEQUENCE_LOCKTIME_TYPE_FLAG (1U << 22)
+#define LOCKTIME_THRESHOLD          500000000U
 
 #define MAX_STANDARD_P2WSH_STACK_ITEMS 100U
 #define MAX_STANDARD_P2WSH_SCRIPT_SIZE 3600U
@@ -109,7 +109,7 @@
 /**
  * Maximum number of derivation steps for a wallet policy xpub (BIP-388).
  */
-#define MAX_BIP388_XPUB_DERIVATION_STEPS 8
+#define MAX_BIP388_XPUB_DERIVATION_STEPS 8U
 
 /**
  * Maximum number of derivation steps allowed for SIGN_PSBT operations,
@@ -132,10 +132,10 @@ _Static_assert(MAX_BIP32_PATH_STEPS == MAX_BIP32_PATH,
  * Index of first hardened child according to BIP32; it can also be used as the bitmask for hardened
  * children.
  */
-#define BIP32_FIRST_HARDENED_CHILD 0x80000000
+#define BIP32_FIRST_HARDENED_CHILD 0x80000000U
 
-#define MAX_BIP44_ACCOUNT_RECOMMENDED       100
-#define MAX_BIP44_ADDRESS_INDEX_RECOMMENDED 50000
+#define MAX_BIP44_ACCOUNT_RECOMMENDED       100U
+#define MAX_BIP44_ADDRESS_INDEX_RECOMMENDED 50000U
 
 // Upper bound of Bitcoin's total supply in satoshis
 #define BITCOIN_TOTAL_SUPPLY (21000000ULL * 100000000UL)

### src/handler/sign_psbt/amount_from_psbt.c
@@ -114,9 +114,9 @@ int __attribute__((noinline)) get_amount_scriptpubkey_from_psbt_witness(
     if (wit_utxo_len < 8 + 1) {
         return -1;
     }
-    int wit_utxo_scriptPubkey_len = raw_witnessUtxo[8];
+    size_t wit_utxo_scriptPubkey_len = raw_witnessUtxo[8];
 
-    if (wit_utxo_len != 8 + 1 + wit_utxo_scriptPubkey_len) {
+    if ((size_t) wit_utxo_len != 8 + 1 + wit_utxo_scriptPubkey_len) {
         PRINTF("Length mismatch for witness utxo's scriptPubKey\n");
         return -1;
     }

### src/handler/sign_psbt/preprocess_outputs.c
@@ -104,7 +104,7 @@ bool __attribute__((noinline)) preprocess_outputs(
 
     // the counter used when showing outputs to the user, which ignores change outputs
     // (0-indexed here, although the UX starts with 1)
-    int external_outputs_count = 0;
+    unsigned int external_outputs_count = 0;
 
     for (unsigned int cur_output_index = 0; cur_output_index < st->n_outputs; cur_output_index++) {
         output_info_t output;

### src/handler/sign_psbt/process_in_outs.c
@@ -65,7 +65,7 @@ int read_change_and_index_from_psbt_bip32_derivation(
         return -1;
     }
 
-    if (der_len < 2 || der_len > MAX_BIP32_PATH_STEPS) {
+    if (der_len < 2 || (unsigned int) der_len > MAX_BIP32_PATH_STEPS) {
         PRINTF("BIP32_DERIVATION path too long\n");
         return 0;
     }

### src/handler/sign_psbt/swap_checks.c
@@ -178,7 +178,7 @@ bool __attribute__((noinline)) execute_swap_checks(dispatcher_context_t *dc,
         finalize_exchange_sign_transaction(false);
     }
 
-    LEDGER_ASSERT(0 <= swap_dest_idx && swap_dest_idx < N_CACHED_EXTERNAL_OUTPUTS,
+    LEDGER_ASSERT(0 <= swap_dest_idx && (unsigned int) swap_dest_idx < N_CACHED_EXTERNAL_OUTPUTS,
                   "External output index out of range for swap\n");
 
     // Check that total amount and fees are as expected

### src/handler/sign_psbt/transaction_display.c
@@ -122,7 +122,7 @@ static bool __attribute__((noinline)) display_external_outputs(
 
     // the counter used when showing outputs to the user, which ignores change outputs
     // (0-indexed here, although the UX starts with 1)
-    int external_outputs_count = 0;
+    unsigned int external_outputs_count = 0;
 
     for (unsigned int cur_output_index = 0; cur_output_index < st->n_outputs; cur_output_index++) {
         if (!bitvector_get(internal_outputs, cur_output_index)) {

### unit-tests/test_wallet.c
@@ -368,6 +368,25 @@ static void test_parse_unsigned_decimal_overflow(void **state) {
     assert_true(0 > parse_policy("wsh(older(5368709120))", out, sizeof(out)));
 }
 
+// Regression test: in a /<M;N>/* key expression, M and N must be unhardened.
+// An older version incorrectly accepted 0x80000000 itself (the first hardened index) as a valid
+// unhardened value.
+static void test_parse_keyexpr_multipath_hardened_boundary(void **state) {
+    (void) state;
+
+    uint8_t out[MAX_WALLET_POLICY_MEMORY_SIZE];
+
+    // 0x7fffffff (2147483647) is the largest valid unhardened index: still accepted.
+    int res = parse_policy("pkh(@0/<2147483647;0>/*)", out, sizeof(out));
+    assert_true(res >= 0);
+    policy_node_with_key_t *node_1 = (policy_node_with_key_t *) out;
+    check_key_expr_plain(r_policy_node_keyexpr(&node_1->key), 0, 2147483647, 0);
+
+    // 0x80000000 (2147483648) is the first hardened index: must be rejected for both M and N.
+    assert_true(0 > parse_policy("pkh(@0/<2147483648;0>/*)", out, sizeof(out)));
+    assert_true(0 > parse_policy("pkh(@0/<0;2147483648>/*)", out, sizeof(out)));
+}
+
 static void test_failures(void **state) {
     (void) state;
 
@@ -1036,6 +1055,7 @@ int main() {
         cmocka_unit_test(test_parse_policy_tr_musig_keypath),
         cmocka_unit_test(test_get_policy_segwit_version),
         cmocka_unit_test(test_parse_unsigned_decimal_overflow),
+        cmocka_unit_test(test_parse_keyexpr_multipath_hardened_boundary),
         cmocka_unit_test(test_failures),
         cmocka_unit_test(test_miniscript_types),
         cmocka_unit_test(test_traverse_single_leaf),