Merge pull request #595 from LedgerHQ/feat/apa/permit_amount_filtering

Permit (ERC-2612) amount-join filtering

client/src/ledger_app_clients/ethereum/eip712/InputData.py

@@ -203,7 +203,12 @@ def send_struct_impl_field(value, field):
             if filtering_paths[path]["type"] == "amount_join_token":
                 send_filtering_amount_join_token(filtering_paths[path]["token"])
             elif filtering_paths[path]["type"] == "amount_join_value":
-                send_filtering_amount_join_value(filtering_paths[path]["token"],
+                if "token" in filtering_paths[path].keys():
+                    token = filtering_paths[path]["token"]
+                else:
+                    # Permit (ERC-2612)
+                    token = 0xff
+                send_filtering_amount_join_value(token,
                                                  filtering_paths[path]["name"])
             elif filtering_paths[path]["type"] == "datetime":
                 send_filtering_datetime(filtering_paths[path]["name"])

doc/ethapp.adoc

@@ -850,6 +850,8 @@ The signature is computed on :
 
 This command should come before the corresponding *SEND STRUCT IMPLEMENTATION* and are only usable for message fields (and not domain ones).
 
+A token index of 0xFF indicates the token address is in the _verifyingContract_ field of the EIP712Domain so the app won't receive an amount-join token filtering APDU. This enables support for Permit (ERC-2612) messages.
+
 The signature is computed on :
 
 22 || chain ID (BE) || contract address || schema hash || field path || display name || token index

src/manage_asset_info.c

@@ -6,33 +6,35 @@ void forget_known_assets(void) {
     tmpCtx.transactionContext.currentAssetIndex = 0;
 }
 
-static extraInfo_t *get_asset_info(uint8_t index) {
-    if (index >= MAX_ASSETS) {
+static extraInfo_t *get_asset_info(int index) {
+    if ((index < 0) || (index >= MAX_ASSETS)) {
         return NULL;
     }
     return &tmpCtx.transactionContext.extraInfo[index];
 }
 
-static bool asset_info_is_set(uint8_t index) {
-    if (index >= MAX_ASSETS) {
+static bool asset_info_is_set(int index) {
+    if ((index < 0) || (index >= MAX_ASSETS)) {
         return false;
     }
     return tmpCtx.transactionContext.assetSet[index];
 }
 
-extraInfo_t *get_asset_info_by_addr(const uint8_t *contractAddress) {
+int get_asset_index_by_addr(const uint8_t *addr) {
     // Works for ERC-20 & NFT tokens since both structs in the union have the
     // contract address aligned
-    for (uint8_t i = 0; i < MAX_ASSETS; i++) {
-        extraInfo_t *currentItem = get_asset_info(i);
-        if (asset_info_is_set(i) &&
-            (memcmp(currentItem->token.address, contractAddress, ADDRESS_LENGTH) == 0)) {
+    for (int i = 0; i < MAX_ASSETS; i++) {
+        extraInfo_t *asset = get_asset_info(i);
+        if (asset_info_is_set(i) && (memcmp(asset->token.address, addr, ADDRESS_LENGTH) == 0)) {
             PRINTF("Token found at index %d\n", i);
-            return currentItem;
+            return i;
         }
     }
+    return -1;
+}
 
-    return NULL;
+extraInfo_t *get_asset_info_by_addr(const uint8_t *addr) {
+    return get_asset_info(get_asset_index_by_addr(addr));
 }
 
 extraInfo_t *get_current_asset_info(void) {

src/manage_asset_info.h

@@ -1,8 +1,14 @@
+#ifndef MANAGE_ASSET_INFO_H_
+#define MANAGE_ASSET_INFO_H_
+
 #include "shared_context.h"
 #include "common_utils.h"
 #include "asset_info.h"
 
 void forget_known_assets(void);
+int get_asset_index_by_addr(const uint8_t *addr);
 extraInfo_t *get_asset_info_by_addr(const uint8_t *contractAddress);
 extraInfo_t *get_current_asset_info(void);
 void validate_current_asset_info(void);
+
+#endif  // MANAGE_ASSET_INFO_H_

src_features/signMessageEIP712/filtering.c

@@ -5,6 +5,7 @@
 #include "ethUstream.h"      // INT256_LENGTH
 #include "apdu_constants.h"  // APDU return codes
 #include "public_keys.h"
+#include "manage_asset_info.h"
 #include "context_712.h"
 #include "commands_712.h"
 #include "typed_data.h"
@@ -17,6 +18,8 @@
 #define FILT_MAGIC_DATETIME          33
 #define FILT_MAGIC_RAW_FIELD         72
 
+#define TOKEN_IDX_ADDR_IN_DOMAIN 0xff
+
 /**
  * Reconstruct the field path and hash it
  *
@@ -386,6 +389,19 @@ bool filtering_amount_join_value(const uint8_t *payload, uint8_t length) {
     }
 
     // Handling
+    if (token_idx == TOKEN_IDX_ADDR_IN_DOMAIN) {
+        // Permit (ERC-2612)
+        int resolved_idx = get_asset_index_by_addr(eip712_context->contract_addr);
+
+        if (resolved_idx == -1) {
+            PRINTF("ERROR: Could not find asset info for verifyingContract address!\n");
+            return false;
+        }
+        token_idx = (uint8_t) resolved_idx;
+        // simulate as if we had received a token-join addr
+        ui_712_token_join_prepare_addr_check(token_idx);
+        amount_join_set_token_received();
+    }
     if (!check_typename("uint") || !check_token_index(token_idx)) {
         return false;
     }

src_features/signMessageEIP712/ui_logic.c

@@ -397,6 +397,13 @@ static bool ui_712_format_amount_join(void) {
     return true;
 }
 
+/**
+ * Simply mark the current amount-join's token address as received
+ */
+void amount_join_set_token_received(void) {
+    ui_ctx->amount.joins[ui_ctx->amount.idx].flags |= AMOUNT_JOIN_FLAG_TOKEN;
+}
+
 /**
  * Update the state of the amount-join
  *
@@ -413,7 +420,7 @@ static bool update_amount_join(const uint8_t *data, uint8_t length) {
             if (memcmp(data, token->address, ADDRESS_LENGTH) != 0) {
                 return false;
             }
-            ui_ctx->amount.joins[ui_ctx->amount.idx].flags |= AMOUNT_JOIN_FLAG_TOKEN;
+            amount_join_set_token_received();
             break;
 
         case AMOUNT_JOIN_STATE_VALUE:

src_features/signMessageEIP712/ui_logic.h

@@ -37,6 +37,7 @@ void ui_712_queue_struct_to_review(void);
 void ui_712_notify_filter_change(void);
 void ui_712_token_join_prepare_addr_check(uint8_t index);
 void ui_712_token_join_prepare_amount(uint8_t index, const char *name, uint8_t name_length);
+void amount_join_set_token_received(void);
 
 #endif  // HAVE_EIP712_FULL_SUPPORT
 

tests/ragger/test_eip712.py

@@ -180,61 +180,53 @@ def test_eip712_new(firmware: Firmware,
     assert recovered_addr == get_wallet_addr(app_client)
 
 
-def test_eip712_advanced_filtering(firmware: Firmware,
-                                   backend: BackendInterface,
-                                   navigator: Navigator,
-                                   default_screenshot_path: Path,
-                                   test_name: str,
-                                   verbose: bool):
-    global SNAPS_CONFIG
-
-    app_client = EthAppClient(backend)
-    if firmware.device == "nanos":
-        pytest.skip("Not supported on LNS")
-
-    if verbose:
-        test_name += "_verbose"
-    SNAPS_CONFIG = SnapshotsConfig(test_name)
-
-    data = {
-        "domain": {
-            "chainId": 1,
-            "name": "Advanced test",
-            "verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC",
-            "version": "1"
-        },
-        "message": {
-            "with": "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045",
-            "value_recv": 10000000000000000,
-            "token_send": "0x6B175474E89094C44Da98b954EedeAC495271d0F",
-            "value_send": 24500000000000000000,
-            "token_recv": "0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2",
-            "expires": 1714559400,
+class DataSet():
+    data: dict
+    filters: dict
+    suffix: str
+
+    def __init__(self, data: dict, filters: dict, suffix: str = ""):
+        self.data = data
+        self.filters = filters
+        self.suffix = suffix
+
+
+ADVANCED_DATA_SETS = [
+    DataSet(
+        {
+            "domain": {
+                "chainId": 1,
+                "name": "Advanced test",
+                "verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC",
+                "version": "1"
+            },
+            "message": {
+                "with": "0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045",
+                "value_recv": 10000000000000000,
+                "token_send": "0x6B175474E89094C44Da98b954EedeAC495271d0F",
+                "value_send": 24500000000000000000,
+                "token_recv": "0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2",
+                "expires": 1714559400,
+            },
+            "primaryType": "Transfer",
+            "types": {
+                "EIP712Domain": [
+                    {"name": "name", "type": "string"},
+                    {"name": "version", "type": "string"},
+                    {"name": "chainId", "type": "uint256"},
+                    {"name": "verifyingContract", "type": "address"}
+                ],
+                "Transfer": [
+                    {"name": "with", "type": "address"},
+                    {"name": "value_recv", "type": "uint256"},
+                    {"name": "token_send", "type": "address"},
+                    {"name": "value_send", "type": "uint256"},
+                    {"name": "token_recv", "type": "address"},
+                    {"name": "expires", "type": "uint64"},
+                ]
+            }
         },
-        "primaryType": "Transfer",
-        "types": {
-            "EIP712Domain": [
-                {"name": "name", "type": "string"},
-                {"name": "version", "type": "string"},
-                {"name": "chainId", "type": "uint256"},
-                {"name": "verifyingContract", "type": "address"}
-            ],
-            "Transfer": [
-                {"name": "with", "type": "address"},
-                {"name": "value_recv", "type": "uint256"},
-                {"name": "token_send", "type": "address"},
-                {"name": "value_send", "type": "uint256"},
-                {"name": "token_recv", "type": "address"},
-                {"name": "expires", "type": "uint64"},
-            ]
-        }
-    }
-
-    if verbose:
-        settings_toggle(firmware, navigator, [SettingID.VERBOSE_EIP712])
-        filters = None
-    else:
-        filters = {
+        {
             "name": "Advanced Filtering",
             "tokens": [
                 {
@@ -279,15 +271,92 @@ def test_eip712_advanced_filtering(firmware: Firmware,
                 },
             }
         }
+    ),
+    DataSet(
+        {
+            "types": {
+                "EIP712Domain": [
+                    {"name": "name", "type": "string"},
+                    {"name": "version", "type": "string"},
+                    {"name": "chainId", "type": "uint256"},
+                    {"name": "verifyingContract", "type": "address"},
+                ],
+                "Permit": [
+                    {"name": "owner", "type": "address"},
+                    {"name": "spender", "type": "address"},
+                    {"name": "value", "type": "uint256"},
+                    {"name": "nonce", "type": "uint256"},
+                    {"name": "deadline", "type": "uint256"},
+                ]
+            },
+            "primaryType": "Permit",
+            "domain": {
+                "name": "ENS",
+                "version": "1",
+                "verifyingContract": "0xC18360217D8F7Ab5e7c516566761Ea12Ce7F9D72",
+                "chainId": 1,
+            },
+            "message": {
+                "owner": "0xb5a6948372defdfc5754b69dc831d21e2d5ebd74",
+                "spender": "0x5B38Da6a701c568545dCfcB03FcB875f56beddC4",
+                "value": 4200000000000000000,
+                "nonce": 0,
+                "deadline": 1719756000,
+            }
+        },
+        {
+            "name": "Permit filtering",
+            "tokens": [
+                {
+                    "addr": "0xC18360217D8F7Ab5e7c516566761Ea12Ce7F9D72",
+                    "ticker": "ENS",
+                    "decimals": 18,
+                    "chain_id": 1,
+                },
+            ],
+            "fields": {
+                "value": {
+                    "type": "amount_join_value",
+                    "name": "Send",
+                },
+                "deadline": {
+                    "type": "datetime",
+                    "name": "Deadline",
+                },
+            }
+        },
+        "_permit"
+    ),
+]
+
+
+@pytest.fixture(name="data_set", params=ADVANCED_DATA_SETS)
+def data_set_fixture(request) -> DataSet:
+    return request.param
+
+
+def test_eip712_advanced_filtering(firmware: Firmware,
+                                   backend: BackendInterface,
+                                   navigator: Navigator,
+                                   default_screenshot_path: Path,
+                                   test_name: str,
+                                   data_set: DataSet):
+    global SNAPS_CONFIG
+
+    app_client = EthAppClient(backend)
+    if firmware.device == "nanos":
+        pytest.skip("Not supported on LNS")
+
+    SNAPS_CONFIG = SnapshotsConfig(test_name + data_set.suffix)
 
     vrs = eip712_new_common(firmware,
                             navigator,
                             default_screenshot_path,
                             app_client,
-                            data,
-                            filters,
-                            verbose)
+                            data_set.data,
+                            data_set.filters,
+                            False)
 
     # verify signature
-    addr = recover_message(data, vrs)
+    addr = recover_message(data_set.data, vrs)
     assert addr == get_wallet_addr(app_client)