Merge pull request #9 from SiaFoundation/audit-fixes

Audit fixes

.clang-format

@@ -10,7 +10,6 @@ SortIncludes: false
 SpaceAfterCStyleCast: true
 AllowShortCaseLabelsOnASingleLine: false
 AllowAllArgumentsOnNextLine: false
-AllowAllParametersOfDeclarationOnNextLine: false
 AllowShortBlocksOnASingleLine: Never
 AllowShortFunctionsOnASingleLine: None
 BinPackArguments: false

Makefile

@@ -68,7 +68,7 @@ VARIANT_PARAM = COIN
 VARIANT_VALUES = sia
 
 # Enabling DEBUG flag will enable PRINTF and disable optimizations
-#DEBUG = 1
+# DEBUG = 1
 
 ########################################
 #     Application custom permissions   #

README.md

@@ -18,10 +18,10 @@ your Ledger device, follow Ledger's [setup instructions](https://speculos.ledger
 After the app is installed, build the `sialedger.go` binary to interact with
 the device. `./sialedger --help` will print a list of commands.
 
-## Usage
+## Installation and Usage
 
 Please refer to our [standalone guide](https://docs.sia.tech/sia-integrations/using-the-sia-ledger-nano-app-sia-central) for a walkthrough that demonstrates how
-to generate addresses and sign transactions using the app.
+to install the app, generate addresses and sign transactions.
 
 ## Security Model
 

TUTORIAL.md

@@ -1,57 +1,3 @@
-## Introduction
+## Tutorial
 
-This is a guide on how to use the Sia app on a Ledger Nano device.  The Sia app currently supports the Ledger Stax, Nano S, Nano SP, and Nano X models and supports transactions involving Siacoins and Siafunds.
-
-## Requirements
-
-    - Initialized Ledger device with the latest firmware
-    - Ledger device connected to computer via USB
-    - Ledger Live open
-
-## Enable Developer Mode
-The Sia app for Ledger devices can only be installed in developer mode.  To enable developer mode, do the following:
-
-1. Open the **Settings** tab in Ledger Live
-2. Select the **Experimental Features** tab within settings
-3. Enable the option titled **Developer Mode**
-
-## Install the Sia App
-
-1. Select the **Manager** tab in Ledger Live
-2. Ensure you are on the **Apps Catalog** tab within the manager
-3. If prompted, allow the Ledger Manager access to your device
-4. Search for Sia within the app catalog
-5. Click install and wait for processing to complete
-6. Exit Ledger Live
-
-## Connecting to Sia Wallet
-
-1. Connect and unlock your Ledger device
-2. Ensure that Ledger Live does not have the manager open.  Leaving it open will interfere with the Sia web wallet and cause the Sia app to crash.
-3. Open the Sia app on the device
-4. Visit the [Sia Central Wallet website](https://wallet.siacentral.com/)
-5. When we first visit the Sia Central Wallet, we are prompted to set a password to encrypt the wallet(s) with.  Make sure to select a secure password.
-6. After typing and confirming the password, select **Ledger Wallet** as the type of wallet.
-7. Press the connect button and select your Ledger device
-8. Select **Import Public Key** and accept the public key request on your device
-9. Press done to confirm wallet importation
-
-## Using Sia Wallet
-
-### Viewing Balance
-
-The default screen of the Sia Central Wallet allows you to see your balance along with its value in USD.
-
-### Sending
-
-Press the **Send** button.  Then input the recipient address, along with the amount, denominated in either SC or USD.  Press **Send**, then confirm the transaction information on your Ledger device and then press accept to complete the signing.
-
-### Receiving
-
-Press the **Receive** button.  The address of your wallet represented in text and as a QR code will appear.
-
-## Finding Support / Communities
-
-- [Discord](https://discord.gg/sFCT3Ar)
-- [Forum](https://forum.sia.tech/)
-- [Sia Docs](https://support.sia.tech/)
+Please refer to https://docs.sia.tech/sia-integrations/using-the-sia-ledger-nano-app-sia-central for the latest guide on installing and using the Ledger app for Sia.

docs/apdu.md

@@ -12,8 +12,8 @@ All commands use CLA = 0xE0.
 | ---- | ---- | -------------- | --------------------------------------- |
 | 0xE0 | 0x01 | GET_VERSION    | Returns version of the app              |
 | 0xE0 | 0x02 | GET_PUBLIC_KEY | Returns public key or addreses          |
-| 0xE0 | 0x03 | SIGN_HASH      | Sign a 32 byte hash                     |
-| 0xE0 | 0x04 | GET_TXN_HASH   | Sign a transaction or retrieve its hash |
+| 0xE0 | 0x04 | SIGN_HASH      | Sign a 32 byte hash                     |
+| 0xE0 | 0x08 | GET_TXN_HASH   | Sign a transaction or retrieve its hash |
 
 ### Commands requiring multiple messages
 

src/app_main.c

@@ -33,11 +33,22 @@
 // because multiple files include ux.h; they need to be defined in exactly one
 // place. See ux.h for their descriptions.
 commandContext global;
+const internalStorage_t N_storage_real;
 
 void ui_idle(void);
 void ui_menu_about(void);
 
+static void update_blind_sign_ui(void);
+
+static void toggle_blind_sign(void) {
+    const bool new_value = !N_storage.blindSign;
+    nvm_write((void *) &N_storage.blindSign, (void *) &new_value, sizeof(bool));
+    update_blind_sign_ui();
+}
+
 #ifdef HAVE_BAGL
+static char BLIND_SIGNING_MESSAGE[22] = {0};
+
 UX_STEP_NOCB(ux_menu_ready_step, nn, {"Awaiting", "commands"});
 UX_STEP_CB(ux_menu_about_step, pn, ui_menu_about(), {&C_icon_certificate, "About"});
 UX_STEP_VALID(ux_menu_exit_step, pn, os_sched_exit(0), {&C_icon_dashboard, "Quit"});
@@ -50,14 +61,20 @@ UX_FLOW(ux_menu_main_flow, &ux_menu_ready_step, &ux_menu_about_step, &ux_menu_ex
 
 UX_STEP_NOCB(ux_menu_version_step, bn, {"Version", APPVERSION});
 UX_STEP_NOCB(ux_menu_developer_step, bn, {"Developer", APPDEVELOPER});
+UX_STEP_CB(ux_menu_blind_sign_step,
+           pn,
+           toggle_blind_sign(),
+           {&C_icon_certificate, BLIND_SIGNING_MESSAGE});
 UX_STEP_CB(ux_menu_back_step, pb, ui_idle(), {&C_icon_back, "Back"});
 
 // flow for the about submenu:
 // #1 screen: app version
-// #2 screen: back button
+// #2 screen: blind sign setting
+// #3 screen: back button
 UX_FLOW(ux_menu_about_flow,
         &ux_menu_version_step,
         &ux_menu_developer_step,
+        &ux_menu_blind_sign_step,
         &ux_menu_back_step,
         FLOW_LOOP);
 
@@ -72,17 +89,58 @@ void ui_idle(void) {
 void ui_menu_about(void) {
     ux_flow_init(0, ux_menu_about_flow, NULL);
 }
+
+static void update_blind_sign_ui(void) {
+    if (N_storage.blindSign) {
+        memcpy(BLIND_SIGNING_MESSAGE, "Disable blind signing", sizeof(BLIND_SIGNING_MESSAGE));
+    } else {
+        memcpy(BLIND_SIGNING_MESSAGE, "Enable blind signing", sizeof(BLIND_SIGNING_MESSAGE));
+    }
+    ui_idle();
+}
+
 #else
-static const char *const INFO_TYPES[] = {"Version", "Developer"};
-static const char *const INFO_CONTENTS[] = {APPVERSION, APPDEVELOPER};
 
-static bool nav_callback(uint8_t page, nbgl_pageContent_t *content) {
+static void controls_callback(int token, uint8_t index, int page);
+
+#define SETTING_INFO_NB     2
+static const char *const INFO_TYPES[SETTING_INFO_NB] = {"Version", "Developer"};
+static const char *const INFO_CONTENTS[SETTING_INFO_NB] = {APPVERSION, APPDEVELOPER};
+
+// settings switches definitions
+enum { BLIND_SIGNING_TOKEN = FIRST_USER_TOKEN };
+enum { BLIND_SIGNING_ID = 0, SETTINGS_SWITCHES_NB };
+
+static nbgl_contentSwitch_t switches[SETTINGS_SWITCHES_NB] = {0};
+
+static const nbgl_contentInfoList_t infoList = {
+    .nbInfos = SETTING_INFO_NB,
+    .infoTypes = INFO_TYPES,
+    .infoContents = INFO_CONTENTS,
+};
+
+// settings menu definition
+#define SETTING_CONTENTS_NB 1
+static const nbgl_content_t contents[SETTING_CONTENTS_NB] = {
+    {.type = SWITCHES_LIST,
+     .content.switchesList.nbSwitches = SETTINGS_SWITCHES_NB,
+     .content.switchesList.switches = switches,
+     .contentActionCallback = controls_callback}};
+
+static const nbgl_genericContents_t settingContents = {.callbackCallNeeded = false,
+                                                       .contentsList = contents,
+                                                       .nbContents = SETTING_CONTENTS_NB};
+
+static void controls_callback(int token, uint8_t index, int page) {
+    UNUSED(index);
     UNUSED(page);
-    content->type = INFOS_LIST;
-    content->infosList.nbInfos = 2;
-    content->infosList.infoTypes = INFO_TYPES;
-    content->infosList.infoContents = INFO_CONTENTS;
-    return true;
+    if (token == BLIND_SIGNING_TOKEN) {
+        toggle_blind_sign();
+    }
+}
+
+static void update_blind_sign_ui(void) {
+    switches[BLIND_SIGNING_ID].initState = N_storage.blindSign ? ON_STATE : OFF_STATE;
 }
 
 void app_quit(void) {
@@ -91,11 +149,19 @@ void app_quit(void) {
 }
 
 void ui_idle(void) {
-    nbgl_useCaseHome(APPNAME, &C_stax_app_sia, NULL, false, ui_menu_about, app_quit);
-}
+    switches[BLIND_SIGNING_ID].text = "Enable blind signing";
+    switches[BLIND_SIGNING_ID].subText = "Recommend only for experienced users";
+    switches[BLIND_SIGNING_ID].token = BLIND_SIGNING_TOKEN;
+    switches[BLIND_SIGNING_ID].tuneId = TUNE_TAP_CASUAL;
 
-void ui_menu_about(void) {
-    nbgl_useCaseSettings(APPNAME, 0, 1, false, ui_idle, nav_callback, NULL);
+    nbgl_useCaseHomeAndSettings(APPNAME,
+                                &C_stax_app_sia,
+                                NULL,
+                                INIT_HOME_PAGE,
+                                &settingContents,
+                                &infoList,
+                                NULL,
+                                app_quit);
 }
 
 #endif
@@ -183,6 +249,12 @@ void app_main() {
     // Initialize io
     io_init();
 
+    if (!N_storage.initialized) {
+        internalStorage_t storage = {0};
+        storage.initialized = true;
+        nvm_write((void *) &N_storage, (void *) &storage, sizeof(internalStorage_t));
+    }
+    update_blind_sign_ui();
     ui_idle();
 
     int input_len = 0;

src/sia_ux.h

@@ -57,6 +57,14 @@ typedef union {
 } commandContext;
 extern commandContext global;
 
+typedef struct internalStorage_t {
+    bool blindSign;
+    bool initialized;
+} internalStorage_t;
+
+extern const internalStorage_t N_storage_real;
+#define N_storage (*(volatile internalStorage_t *) PIC(&N_storage_real))
+
 // ui_idle displays the main menu screen. Command handlers should call ui_idle
 // when they finish.
 void ui_idle(void);

src/signHash.c

@@ -109,6 +109,8 @@ uint16_t handleSignHash(uint8_t p1 __attribute__((unused)),
                         uint16_t len) {
     if (len != sizeof(uint32_t) + SIA_HASH_SIZE) {
         return SW_INVALID_PARAM;
+    } else if (!N_storage.blindSign) {
+        return SW_USER_REJECTED;
     }
 
     // Read the index of the signing key. U4LE is a helper macro for

src/txn.c

@@ -242,9 +242,11 @@ static void __txn_next_elem(txn_state_t *txn) {
             }
 
             txn->sliceIndex++;
-            txn->elements[txn->elementIndex + 1].elemType =
-                txn->elements[txn->elementIndex].elemType;
             txn->elementIndex++;
+            if (txn->elementIndex < MAX_ELEMS) {
+                txn->elements[txn->elementIndex].elemType =
+                    txn->elements[txn->elementIndex - 1].elemType;
+            }
             return;
 
         case TXN_ELEM_SF_OUTPUT:
@@ -254,9 +256,11 @@ static void __txn_next_elem(txn_state_t *txn) {
             advance(txn);
 
             txn->sliceIndex++;
-            txn->elements[txn->elementIndex + 1].elemType =
-                txn->elements[txn->elementIndex].elemType;
             txn->elementIndex++;
+            if (txn->elementIndex < MAX_ELEMS) {
+                txn->elements[txn->elementIndex].elemType =
+                    txn->elements[txn->elementIndex - 1].elemType;
+            }
             return;
 
         case TXN_ELEM_MINER_FEE:
@@ -265,9 +269,11 @@ static void __txn_next_elem(txn_state_t *txn) {
             advance(txn);
 
             txn->sliceIndex++;
-            txn->elements[txn->elementIndex + 1].elemType =
-                txn->elements[txn->elementIndex].elemType;
             txn->elementIndex++;
+            if (txn->elementIndex < MAX_ELEMS) {
+                txn->elements[txn->elementIndex].elemType =
+                    txn->elements[txn->elementIndex - 1].elemType;
+            }
             return;
 
         // these elements should be decoded, but not displayed

tests/test_app_mainmenu.py

@@ -15,6 +15,7 @@ def test_app_mainmenu(firmware, navigator, test_name):
     else:
         instructions = [
             NavInsID.USE_CASE_HOME_INFO,
+            NavInsID.USE_CASE_SETTINGS_NEXT,
             NavInsID.USE_CASE_SETTINGS_SINGLE_PAGE_EXIT,
         ]
     navigator.navigate_and_compare(

tests/test_sign_hash_cmd.py

@@ -7,7 +7,7 @@
     unpack_sign_tx_response,
 )
 from ragger.backend import RaisePolicy
-from ragger.navigator import NavInsID
+from ragger.navigator import NavIns, NavInsID
 from ragger.bip import calculate_public_key_and_chaincode, CurveChoice
 from utils import ROOT_SCREENSHOT_PATH
 
@@ -20,10 +20,26 @@ def test_sign_hash_accept(firmware, backend, navigator, test_name):
     client = BoilerplateCommandSender(backend)
     index = 5
 
+    if firmware.device.startswith("nano"):
+        navigator.navigate([
+            NavInsID.RIGHT_CLICK,
+            NavInsID.BOTH_CLICK,
+            NavInsID.RIGHT_CLICK,
+            NavInsID.RIGHT_CLICK,
+            NavInsID.BOTH_CLICK,
+        ], screen_change_before_first_instruction=False)
+    else:
+        navigator.navigate([
+            NavInsID.USE_CASE_HOME_SETTINGS,
+            NavIns(NavInsID.TOUCH, (350,115)),
+            NavInsID.USE_CASE_SETTINGS_MULTI_PAGE_EXIT,
+        ], screen_change_before_first_instruction=False)
+
     with client.sign_hash_with_confirmation(index=index, to_sign=test_to_sign):
         # Disable raising when trying to unpack an error APDU
         backend.raise_policy = RaisePolicy.RAISE_NOTHING
         if firmware.device.startswith("nano"):
+            # enable blind signing
             navigator.navigate_until_text_and_compare(
                 NavInsID.RIGHT_CLICK,
                 [NavInsID.BOTH_CLICK],
@@ -53,6 +69,21 @@ def test_sign_hash_reject(firmware, backend, navigator, test_name):
     client = BoilerplateCommandSender(backend)
     index = 5
 
+    if firmware.device.startswith("nano"):
+        navigator.navigate([
+            NavInsID.RIGHT_CLICK,
+            NavInsID.BOTH_CLICK,
+            NavInsID.RIGHT_CLICK,
+            NavInsID.RIGHT_CLICK,
+            NavInsID.BOTH_CLICK,
+        ], screen_change_before_first_instruction=False)
+    else:
+        navigator.navigate([
+            NavInsID.USE_CASE_HOME_SETTINGS,
+            NavIns(NavInsID.TOUCH, (350,115)),
+            NavInsID.USE_CASE_SETTINGS_MULTI_PAGE_EXIT,
+        ], screen_change_before_first_instruction=False)
+
     with client.sign_hash_with_confirmation(index=index, to_sign=test_to_sign):
         # Disable raising when trying to unpack an error APDU
         backend.raise_policy = RaisePolicy.RAISE_NOTHING
@@ -67,7 +98,9 @@ def test_sign_hash_reject(firmware, backend, navigator, test_name):
             )
         else:
             navigator.navigate_and_compare(
-                ROOT_SCREENSHOT_PATH, test_name, [NavInsID.USE_CASE_REVIEW_REJECT]
+                ROOT_SCREENSHOT_PATH, test_name, [
+                    NavInsID.USE_CASE_REVIEW_REJECT,
+                ]
             )
 
     # Assert that we have received a refusal