Merge pull request #238 from LedgerHQ/fbe/disallow_extension_on_legac…

…y_flows Revert "Allow extension feature on legacy flows"
LedgerHQ · Dec 5, 2024 · b1e6989 · b1e6989
2 parents dc60e76 + 80a0ad0
commit b1e6989
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 3 deletions.
diff --git a/doc/protocol.md b/doc/protocol.md
@@ -88,6 +88,8 @@ Changing the subcommand after an exchange flow is started will result in an erro
 
 ### EXTENSION:
 
+This feature is only available in a PROCESS_TRANSACTION_RESPONSE command in a SWAP_NEW, SELL_NEW, or FUND_NEW flow.\
+In Legacy flows the extension must be P2_NONE. \
 The extension is sent to the app as the upper 4 bits of the P2 byte of the APDU.
 
 The maximum DATA in a single APDU is 255 bytes, in case it is not sufficient for a command,

diff --git a/src/apdu_parser.c b/src/apdu_parser.c
@@ -215,7 +215,11 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
     bool is_first_data_chunk = !(extension & P2_EXTEND);
     bool is_last_data_chunk = !(extension & P2_MORE);
     bool is_whole_apdu = is_first_data_chunk && is_last_data_chunk;
-
+    // Split reception is only for NG flows
+    if (subcommand != SWAP_NG && subcommand != SELL_NG && subcommand != FUND_NG && !is_whole_apdu) {
+        PRINTF("Extension %d refused, only allowed for unified flows\n", extension);
+        return WRONG_P2_EXTENSION;
+    }
     // Split reception is only for PROCESS_TRANSACTION_RESPONSE_COMMAND
     if (instruction != PROCESS_TRANSACTION_RESPONSE_COMMAND && !is_whole_apdu) {
         PRINTF("Extension %d refused, only allowed for PROCESS_TRANSACTION_RESPONSE instruction\n",

diff --git a/test/python/test_extension.py b/test/python/test_extension.py
@@ -52,6 +52,23 @@
 
 class TestExtension:
 
+    @pytest.mark.parametrize("subcommand", LEGACY_SUBCOMMANDS)
+    def test_extension_forbidden_for_subcommand(self, backend, subcommand):
+        ex = ExchangeClient(backend, Rate.FIXED, subcommand)
+        partner = SigningAuthority(curve=get_partner_curve(subcommand), name="Name")
+        transaction_id = ex.init_transaction().data
+        credentials = get_credentials(subcommand, partner)
+        ex.set_partner_key(credentials)
+        ex.check_partner_key(LEDGER_SIGNER.sign(credentials))
+        tx, _ = craft_and_sign_tx(subcommand, TX_INFOS[subcommand], transaction_id, FEES, partner)
+        # Send the exchange transaction proposal and its signature
+
+        for ext in [P2_MORE, P2_EXTEND, (P2_EXTEND | P2_MORE)]:
+            with pytest.raises(ExceptionRAPDU) as e:
+                backend.exchange(ex.CLA, Command.PROCESS_TRANSACTION_RESPONSE, p1=Rate.FIXED, p2=(subcommand | ext), data=tx)
+            assert e.value.status == Errors.WRONG_P2_EXTENSION
+
+
     @pytest.mark.parametrize("subcommand", ALL_SUBCOMMANDS)
     def test_extension_forbidden_for_command(self, backend, subcommand):
         for ext in [P2_MORE, P2_EXTEND, (P2_EXTEND | P2_MORE)]:
@@ -60,7 +77,7 @@ def test_extension_forbidden_for_command(self, backend, subcommand):
             assert e.value.status == Errors.WRONG_P2_EXTENSION
 
 
-    @pytest.mark.parametrize("subcommand", ALL_SUBCOMMANDS)
+    @pytest.mark.parametrize("subcommand", NEW_SUBCOMMANDS)
     def test_extension_many_chunks(self, backend, subcommand):
         ex = ExchangeClient(backend, Rate.FIXED, subcommand)
         partner = SigningAuthority(curve=get_partner_curve(subcommand), name="Name")
@@ -82,7 +99,7 @@ def test_extension_many_chunks(self, backend, subcommand):
         ex.check_transaction_signature(tx_signature)
 
 
-    @pytest.mark.parametrize("subcommand", ALL_SUBCOMMANDS)
+    @pytest.mark.parametrize("subcommand", NEW_SUBCOMMANDS)
     def test_extension_advanced_usage(self, backend, subcommand):
         ex = ExchangeClient(backend, Rate.FIXED, subcommand)
         partner = SigningAuthority(curve=get_partner_curve(subcommand), name="Name")