Fix fees overwrite in starkware plugin (#198)

* increase display buffer size to handle Starkware master key and control size when using snprintf

* Bump version 1.9.6

* Fix broken tests

* Add first starkware tests

* Add some constants for starkware

* Update CHANGELOG.md

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [1.9.6](https://github.com/ledgerhq/app-ethereum/compare/1.9.5...1.9.6) - 2021-9-29
+
+### Fixed
+
+- Fixed a bug where fees displayed were wrong on Starkware transactions
+
 ## [1.9.5](https://github.com/ledgerhq/app-ethereum/compare/1.9.4...1.9.5) - 2021-9-27
 
 ### Changed

Makefile

@@ -30,7 +30,7 @@ APP_LOAD_PARAMS += --path "1517992542'/1101353413'"
 
 APPVERSION_M=1
 APPVERSION_N=9
-APPVERSION_P=5
+APPVERSION_P=6
 APPVERSION=$(APPVERSION_M).$(APPVERSION_N).$(APPVERSION_P)
 APP_LOAD_FLAGS= --appFlags 0x240 --dep Ethereum:$(APPVERSION)
 

src/shared_context.h

@@ -166,7 +166,7 @@ typedef enum {
 
 typedef struct txStringProperties_t {
     char fullAddress[43];
-    char fullAmount[50];
+    char fullAmount[67];
     char maxFee[50];
     char nonce[8];  // 10M tx per account ought to be enough for everybody
     char network_name[NETWORK_STRING_MAX_SIZE];

src_plugins/starkware/starkware_plugin.c

@@ -167,6 +167,9 @@ typedef struct starkware_parameters_t {
 
 } starkware_parameters_t;
 
+#define STARK_KEY_LENGTH (2 + length * 2 + 1)
+#define VAULT_ID_LENGTH  10
+
 bool is_deversify_contract(const uint8_t *address) {
     uint32_t offset = 0;
     uint8_t i;
@@ -271,12 +274,21 @@ bool starkware_verify_nft_token_id(uint8_t *tokenId) {
     return true;
 }
 
-void starkware_print_vault_id(uint32_t vaultId, char *destination) {
+void starkware_print_vault_id(uint32_t vaultId, char *destination, size_t max_length) {
-    snprintf(destination, 10, "%d", vaultId);
+    if (VAULT_ID_LENGTH > max_length) {
+        os_sched_exit(EXCEPTION_OVERFLOW);
+    }
+    snprintf(destination, max_length, "%d", vaultId);
 }
 
-void starkware_print_stark_key(uint8_t *starkKey, char *destination) {
+void starkware_print_stark_key(uint8_t *starkKey,
-    snprintf(destination, 70, "0x%.*H", 32, starkKey);
+                               size_t length,
+                               char *destination,
+                               size_t max_length) {
+    if (STARK_KEY_LENGTH > max_length) {
+        os_sched_exit(EXCEPTION_OVERFLOW);
+    }
+    snprintf(destination, max_length, "0x%.*H", length, starkKey);
 }
 
 // TODO : rewrite as independant code
@@ -741,7 +753,10 @@ void starkware_plugin_call(int message, void *parameters) {
                         case STARKWARE_WITHDRAW_NFT:
                         case STARKWARE_WITHDRAW_NFT_TO:
                             strlcpy(msg->title, "Master Account", msg->titleLength);
-                            starkware_print_stark_key(context->starkKey, msg->msg);
+                            starkware_print_stark_key(context->starkKey,
+                                                      sizeof(context->starkKey),
+                                                      msg->msg,
+                                                      msg->msgLength);
                             break;
                         default:
                             PRINTF("Unexpected screen %d for %d\n",
@@ -758,7 +773,10 @@ void starkware_plugin_call(int message, void *parameters) {
                         case STARKWARE_REGISTER_AND_DEPOSIT_TOKEN:
                         case STARKWARE_REGISTER_AND_DEPOSIT_ETH:
                             strlcpy(msg->title, "Master Account", msg->titleLength);
-                            starkware_print_stark_key(context->starkKey, msg->msg);
+                            starkware_print_stark_key(context->starkKey,
+                                                      sizeof(context->starkKey),
+                                                      msg->msg,
+                                                      msg->msgLength);
                             break;
 
                         case STARKWARE_DEPOSIT_TOKEN:
@@ -772,7 +790,9 @@ void starkware_plugin_call(int message, void *parameters) {
                         case STARKWARE_DEPOSIT_NFT:
                         case STARKWARE_DEPOSIT_NFT_RECLAIM:
                             strlcpy(msg->title, "Token Account", msg->titleLength);
-                            starkware_print_vault_id(U4BE(context->vaultId, 0), msg->msg);
+                            starkware_print_vault_id(U4BE(context->vaultId, 0),
+                                                     msg->msg,
+                                                     msg->msgLength);
                             break;
                         case STARKWARE_WITHDRAW:
                         case STARKWARE_WITHDRAW_NFT:
@@ -806,7 +826,9 @@ void starkware_plugin_call(int message, void *parameters) {
                     switch (context->selectorIndex) {
                         case STARKWARE_ESCAPE:
                             strlcpy(msg->title, "Token Account", msg->titleLength);
-                            starkware_print_vault_id(U4BE(context->vaultId, 0), msg->msg);
+                            starkware_print_vault_id(U4BE(context->vaultId, 0),
+                                                     msg->msg,
+                                                     msg->msgLength);
                             break;
                         case STARKWARE_DEPOSIT_TOKEN:
                         case STARKWARE_DEPOSIT_ETH:
@@ -839,7 +861,9 @@ void starkware_plugin_call(int message, void *parameters) {
                         case STARKWARE_REGISTER_AND_DEPOSIT_TOKEN:
                         case STARKWARE_REGISTER_AND_DEPOSIT_ETH:
                             strlcpy(msg->title, "Token Account", msg->titleLength);
-                            starkware_print_vault_id(U4BE(context->vaultId, 0), msg->msg);
+                            starkware_print_vault_id(U4BE(context->vaultId, 0),
+                                                     msg->msg,
+                                                     msg->msgLength);
                             break;
 
                         default:
@@ -858,7 +882,10 @@ void starkware_plugin_call(int message, void *parameters) {
                         case STARKWARE_DEPOSIT_NFT:
                         case STARKWARE_DEPOSIT_NFT_RECLAIM:
                             strlcpy(msg->title, "TokenID", msg->titleLength);
-                            starkware_print_stark_key(dataContext.tokenContext.quantum, msg->msg);
+                            starkware_print_stark_key(dataContext.tokenContext.quantum,
+                                                      sizeof(dataContext.tokenContext.quantum),
+                                                      msg->msg,
+                                                      msg->msgLength);
                             break;
 
                         case STARKWARE_REGISTER_AND_DEPOSIT_TOKEN:

tests/src/contract_data_warning.test.js

@@ -7,7 +7,7 @@ import Zemu from '@zondax/zemu';
 
 test('[Nano S] Try to blind sign with setting disabled', zemu("nanos", async (sim, eth) => {
   // disable blind signing
-  await sim.navigateAndCompareSnapshots('.', 'nanos_disable_blind_signing', [2, 0, 0, 3, 0]);
+  await sim.navigateAndCompareSnapshots('.', 'nanos_disable_blind_signing', [-2, 0, 0, 3, 0]);
 
   // we can't use eth.signTransaction because it detects that contract data is disabled and fails early
   let transport = await sim.getTransport();
@@ -23,7 +23,7 @@ test('[Nano S] Try to blind sign with setting disabled', zemu("nanos", async (si
 
 test('[Nano X] Try to blind sign with setting disabled', zemu("nanox", async (sim, eth) => {
   // disable blind signing
-  await sim.navigateAndCompareSnapshots('.', 'nanox_disable_blind_signing', [2, 0, 0, 3, 0]);
+  await sim.navigateAndCompareSnapshots('.', 'nanox_disable_blind_signing', [-2, 0, 0, 3, 0]);
 
   // we can't use eth.signTransaction because it detects that contract data is disabled and fails early
   let transport = await sim.getTransport();

tests/src/starkware.test.js

@@ -0,0 +1,65 @@
+import "core-js/stable";
+import "regenerator-runtime/runtime";
+import { waitForAppScreen, zemu } from './test.fixture';
+import { byContractAddressAndChainId } from '@ledgerhq/hw-app-eth/erc20'
+import { BigNumber } from "bignumber.js";
+
+test('[Nano S] Transfer Ether on Ethereum app', zemu("nanos", async (sim, eth) => {
+
+  // Provide USDT token info to the app
+  const usdt_info = byContractAddressAndChainId("0xdac17f958d2ee523a2206206994597c13d831ec7", 1);
+  await eth.provideERC20TokenInformation(usdt_info);
+
+  // Provide Stark quantum
+  const quantization = new BigNumber(1);
+  await eth.starkProvideQuantum_v2(
+    "0xdac17f958d2ee523a2206206994597c13d831ec7",
+    "erc20",
+    quantization,
+    null
+    )
+
+  const tx = eth.signTransaction(
+    "44'/60'/1'/0/0",
+    'f8b5018a0472698b413b43200000825208940102030405060708090a0b0c0d0e0f1011121314872bd72a24874000b8842505c3d9010101010101010102020202020202020303030303030303040404040404040402ce625e94458d39dd0bf3b45a843544dd4a14b8169045a3a3d15aa564b936c500000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000030d40808080',
+  );
+
+  await waitForAppScreen(sim);
+  await sim.navigateAndCompareSnapshots('.', 'nanos_starkware_usdt_deposit', [13, 0]);
+
+  await expect(tx).resolves.toEqual({
+    "r": "14c368c0d32e399470d6113cf796c5f4cd70300766337d8b0ba71ecad21b3d52",
+    "s": "4207c027959e84fc2242a1f4fd955603f137ba28f67268ffc91fef5d65071b0a",
+    "v": "1c",
+  });
+}));
+
+test('[Nano X] Transfer Ether on Ethereum app', zemu("nanox", async (sim, eth) => {
+
+  // Provide USDT token info to the app
+  const usdt_info = byContractAddressAndChainId("0xdac17f958d2ee523a2206206994597c13d831ec7", 1);
+  await eth.provideERC20TokenInformation(usdt_info);
+
+  // Provide Stark quantum
+  const quantization = new BigNumber(1);
+  await eth.starkProvideQuantum_v2(
+    "0xdac17f958d2ee523a2206206994597c13d831ec7",
+    "erc20",
+    quantization,
+    null
+    )
+
+  const tx = eth.signTransaction(
+    "44'/60'/1'/0/0",
+    'f8b5018a0472698b413b43200000825208940102030405060708090a0b0c0d0e0f1011121314872bd72a24874000b8842505c3d9010101010101010102020202020202020303030303030303040404040404040402ce625e94458d39dd0bf3b45a843544dd4a14b8169045a3a3d15aa564b936c500000000000000000000000000000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000030d40808080',
+  );
+
+  await waitForAppScreen(sim);
+  await sim.navigateAndCompareSnapshots('.', 'nanox_starkware_usdt_deposit', [9, 0]);
+
+  await expect(tx).resolves.toEqual({
+    "r": "14c368c0d32e399470d6113cf796c5f4cd70300766337d8b0ba71ecad21b3d52",
+    "s": "4207c027959e84fc2242a1f4fd955603f137ba28f67268ffc91fef5d65071b0a",
+    "v": "1c",
+  });
+}));