wip

.github/workflows/codeql-workflow.yml

@@ -17,7 +17,7 @@ jobs:
     name: Analyse
     strategy:
       matrix:
-        sdk: [ "$NANOS_SDK", "$NANOX_SDK", "$NANOSP_SDK" ]
+        sdk: [ "$NANOX_SDK", "$NANOSP_SDK", "$STAX_SDK", "$FLEX_SDK" ]
         #'cpp' covers C and C++
         language: [ 'cpp' ]
     runs-on: ubuntu-latest

src/apdu_parser.c

@@ -36,13 +36,7 @@ typedef struct apdu_s {
     uint8_t rate;
     uint8_t subcommand;
     uint16_t data_length;
-    // We could have put the reconstructed apdu buffer here but it would increase the RAM usage by
-    // 512 bytes which is a lot on NANOS
-    // Instead the reconstructed apdu buffer is G_swap_ctx.raw_transaction
-    // It is unionized with the decoded protobuf transaction requests
-    // Pro: less memory usage
-    // Cons: use cautiously and only during the command PROCESS_TRANSACTION_RESPONSE_COMMAND
-    // The split reception is only made for this command anyway
+    uint8_t raw_transaction[256 * 2];
 } apdu_t;
 apdu_t G_received_apdu;
 
@@ -252,11 +246,7 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
         G_received_apdu.subcommand = subcommand;
         G_received_apdu.data_length = data_length;
         if (!is_last_data_chunk) {
-            // Use the raw_transaction buffer as temporary storage.
-            // It's unionized with the decoded transaction but we are currently handling
-            // PROCESS_TRANSACTION_RESPONSE_COMMAND instruction (checked previously).
-            // After this command is done it will contain the decoded PB data, don't use it anymore
-            memcpy(G_swap_ctx.raw_transaction, apdu + OFFSET_CDATA, data_length);
+            memcpy(G_received_apdu.raw_transaction, apdu + OFFSET_CDATA, data_length);
             G_received_apdu.expecting_more = true;
         }
     } else {
@@ -277,15 +267,15 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
                    subcommand);
             return INVALID_P2_EXTENSION;
         }
-        if (G_received_apdu.data_length + data_length > sizeof(G_swap_ctx.raw_transaction)) {
+        if (G_received_apdu.data_length + data_length > sizeof(G_received_apdu.raw_transaction)) {
             PRINTF("Reception buffer size %d is not sufficient to receive more data (%d + %d)\n",
-                   sizeof(G_swap_ctx.raw_transaction),
+                   sizeof(G_received_apdu.raw_transaction),
                    G_received_apdu.data_length,
                    data_length);
             return INVALID_P2_EXTENSION;
         }
         // Extend the already received buffer
-        memcpy(G_swap_ctx.raw_transaction + G_received_apdu.data_length,
+        memcpy(G_received_apdu.raw_transaction + G_received_apdu.data_length,
                apdu + OFFSET_CDATA,
                data_length);
         G_received_apdu.data_length += data_length;
@@ -311,7 +301,7 @@ uint16_t check_apdu_validity(uint8_t *apdu, size_t apdu_length, command_t *comma
         } else {
             // Split has taken place, data is in the split buffer
             PRINTF("Split APDU successfully recreated, size %d\n", G_received_apdu.data_length);
-            command->data.bytes = G_swap_ctx.raw_transaction;
+            command->data.bytes = G_received_apdu.raw_transaction;
         }
         return 0;
     }

src/globals.h

@@ -62,8 +62,6 @@ typedef struct swap_app_context_s {
 
     // SWAP, SELL, and FUND flows are unionized as they cannot be used in the same context
     union {
-        // This is the raw received APDU
-        uint8_t raw_transaction[256 * 2];
         ledger_swap_NewTransactionResponse swap_transaction;
         struct {
             ledger_swap_NewSellResponse sell_transaction;

src/process_transaction.c

@@ -336,23 +336,10 @@ static void save_fees(buf_t fees) {
 }
 
 int process_transaction(const command_t *cmd) {
-    uint8_t undecoded_transaction[sizeof(G_swap_ctx.raw_transaction)];
-    uint8_t *data;
-    // For memory optimization, the undecoded protobuf apdu may have been stored in an union
-    // with the decoded protobuf transaction.
-    if (cmd->data.bytes == G_swap_ctx.raw_transaction) {
-        // Copy locally the apdu to avoid problems during protobuf decode and fees extraction
-        PRINTF("Copying locally, the APDU has been received split\n");
-        memcpy(undecoded_transaction, G_swap_ctx.raw_transaction, cmd->data.size);
-        data = undecoded_transaction;
-    } else {
-        data = cmd->data.bytes;
-    }
-
     buf_t fees;
     buf_t payload;
     bool needs_base64_decoding;
-    if (!parse_transaction(data,
+    if (!parse_transaction(cmd->data.bytes,
                            cmd->data.size,
                            cmd->subcommand,
                            &payload,

src/trusted_name_descriptor_handler.c

@@ -266,6 +266,9 @@ static int trusted_name_descriptor_handler_internal(const command_t *cmd) {
                        &trusted_name_info.token_account,
                        &trusted_name_info.owner);
 
+    PRINTF("Payout address : %.*H\n", sizeof(G_swap_ctx.swap_transaction.payout_address), G_swap_ctx.swap_transaction.payout_address);
+    PRINTF("Refund address : %.*H\n", sizeof(G_swap_ctx.swap_transaction.refund_address), G_swap_ctx.swap_transaction.refund_address);
+
     return reply_success();
 }
 

test/python/test_trusted_name.py

@@ -26,8 +26,8 @@
      "payout_extra_id": b"",
      "currency_from": CURRENCY_FROM.ticker,
      "currency_to": CURRENCY_TO.ticker,
-     "amount_to_provider": bytes.fromhex("013fc3a717fb5000"),
-     "amount_to_wallet": b"\x0b\xeb\xc2\x00",
+     "amount_to_provider": bytes.fromhex("01"),
+     "amount_to_wallet": bytes.fromhex("01"),
 }
 FUND_TX_INFOS = {
     "user_id": "John Wick",
@@ -82,8 +82,10 @@ def test_trusted_name_wrong_challenge(self, backend, subcommand):
 
     @pytest.mark.parametrize("subcommand", [SubCommand.SWAP, SubCommand.SWAP_NG])
     @pytest.mark.parametrize("field_to_test", ["payout_address", "refund_address"])
-    @pytest.mark.parametrize("value", [b"0xcafedeca", b"f" * 62])
+    @pytest.mark.parametrize("value", [b"0xcafedeca", b"f" * 150])
     def test_trusted_name_valid(self, backend, subcommand, field_to_test, value):
+        if subcommand == SubCommand.SWAP and len(value) > 50:
+            pytest.skip("Can't test max address value on legacy flows")
         ex = ExchangeClient(backend, Rate.FIXED, subcommand)
         partner = SigningAuthority(curve=get_partner_curve(subcommand), name="Name")
         transaction_id = ex.init_transaction().data
@@ -94,6 +96,7 @@ def test_trusted_name_valid(self, backend, subcommand, field_to_test, value):
         tx_infos = TX_INFOS[subcommand].copy()
         actual_address = tx_infos[field_to_test]
         temp_address = value
+        print(f"Replacing {actual_address} by {temp_address}")
         tx_infos[field_to_test] = temp_address
         tx, tx_signature = craft_and_sign_tx(subcommand, tx_infos, transaction_id, FEES, partner)
         ex.process_transaction(tx)
@@ -178,21 +181,17 @@ def test_trusted_name_wrong_field(self, backend, subcommand):
         ex.check_transaction_signature(tx_signature)
 
         for fake in ["structure_type", "version", "trusted_name_type", "trusted_name_source", "signer_algo"]:
+            challenge = ex.get_challenge().data
             with pytest.raises(ExceptionRAPDU) as e:
                 if fake == "structure_type":
-                    challenge = ex.get_challenge().data
                     ex.send_pki_certificate_and_trusted_name_descriptor(challenge=challenge, structure_type=0)
                 elif fake == "version":
-                    challenge = ex.get_challenge().data
                     ex.send_pki_certificate_and_trusted_name_descriptor(challenge=challenge, version=0)
                 elif fake == "trusted_name_type":
-                    challenge = ex.get_challenge().data
                     ex.send_pki_certificate_and_trusted_name_descriptor(challenge=challenge, trusted_name_type=0)
                 elif fake == "trusted_name_source":
-                    challenge = ex.get_challenge().data
                     ex.send_pki_certificate_and_trusted_name_descriptor(challenge=challenge, trusted_name_source=0)
                 elif fake == "signer_algo":
-                    challenge = ex.get_challenge().data
                     ex.send_pki_certificate_and_trusted_name_descriptor(challenge=challenge, signer_algo=0)
             assert e.value.status == Errors.WRONG_TLV_CONTENT
 
@@ -207,13 +206,15 @@ def test_trusted_name_wrong_field(self, backend, subcommand):
         assert e.value.status == Errors.WRONG_TLV_SIGNATURE
 
         for fake in ["trusted_name", "address"]:
-            for value in [b"", b"F"*63]:
+            values = [b""]
+            if subcommand == SubCommand.SWAP_NG:
+                values += [b"F"*151]
+            for value in values:
+                challenge = ex.get_challenge().data
                 with pytest.raises(ExceptionRAPDU) as e:
                     if fake == "trusted_name":
-                        challenge = ex.get_challenge().data
                         ex.send_pki_certificate_and_trusted_name_descriptor(challenge=challenge, trusted_name=value)
                     elif fake == "address":
-                        challenge = ex.get_challenge().data
                         ex.send_pki_certificate_and_trusted_name_descriptor(challenge=challenge, address=value)
                 assert e.value.status == Errors.WRONG_TLV_FORMAT