From 4770b55f4644502e2c53e0669c311f40db5abefc Mon Sep 17 00:00:00 2001 From: Alexandre Paillier Date: Fri, 21 Jun 2024 11:49:42 +0200 Subject: [PATCH] New blind-signing flow for LNSP/LNX --- src/main.c | 1 - src/ui_callbacks.h | 1 - src_bagl/common_ui.c | 2 +- src_bagl/ui_flow.c | 60 ++++++++++++++++-- src_bagl/ui_flow.h | 2 +- src_bagl/ui_flow_signTx.c | 13 ++-- src_features/signTx/cmd_signTx.c | 2 +- src_features/signTx/feature_signTx.h | 4 +- src_features/signTx/logic_signTx.c | 87 +++++++++++--------------- src_features/signTx/ui_common_signTx.c | 3 +- 10 files changed, 106 insertions(+), 69 deletions(-) diff --git a/src/main.c b/src/main.c index 1141ff0..38876d7 100644 --- a/src/main.c +++ b/src/main.c @@ -39,7 +39,6 @@ unsigned char G_io_seproxyhal_spi_buffer[IO_SEPROXYHAL_BUFFER_SIZE_B]; void ui_idle(void); uint32_t set_result_get_publicKey(void); -void finalizeParsing(bool); tmpCtx_t tmpCtx; txContext_t txContext; diff --git a/src/ui_callbacks.h b/src/ui_callbacks.h index 90984f0..ed6a41e 100644 --- a/src/ui_callbacks.h +++ b/src/ui_callbacks.h @@ -24,4 +24,3 @@ unsigned int io_seproxyhal_touch_privacy_cancel(const bagl_element_t *e); void ui_warning_contract_data(void); void io_seproxyhal_send_status(uint32_t sw); -void finalizeParsing(bool direct); diff --git a/src_bagl/common_ui.c b/src_bagl/common_ui.c index dc64504..044df68 100644 --- a/src_bagl/common_ui.c +++ b/src_bagl/common_ui.c @@ -13,7 +13,7 @@ void ui_idle(void) { } void ui_warning_contract_data(void) { - ux_flow_init(0, ux_warning_contract_data_flow, NULL); + ux_flow_init(0, ux_blind_signing_flow, NULL); } void ui_display_public_eth2(void) { diff --git a/src_bagl/ui_flow.c b/src_bagl/ui_flow.c index 633a111..48d66af 100644 --- a/src_bagl/ui_flow.c +++ b/src_bagl/ui_flow.c @@ -2,6 +2,7 @@ #include "ui_callbacks.h" #include "common_ui.h" #include "common_utils.h" +#include "feature_signTx.h" #define ENABLED_STR "Enabled" #define DISABLED_STR "Disabled" @@ -214,16 +215,63 @@ UX_STEP_CB( "Blind signing must be enabled in Settings", }); #else +UX_STEP_NOCB( + ux_blind_signing_warning_step, + pbb, + { + &C_icon_warning, + "This transaction", + "cannot be trusted", + }); +UX_STEP_NOCB( + ux_blind_signing_text1_step, + nnnn, + { + "Your Ledger cannot", + "decode this", + "transaction. If you", + "sign it, you could", + }); +UX_STEP_NOCB( + ux_blind_signing_text2_step, + nnnn, + { + "be authorizing", + "malicious actions", + "that can drain your", + "wallet.", + }); +UX_STEP_NOCB( + ux_blind_signing_link_step, + nn, + { + "Learn more:", + "ledger.com/e8", + }); UX_STEP_CB( - ux_warning_contract_data_step, - pnn, - ui_idle(), + ux_blind_signing_accept_step, + pbb, + start_signature_flow(), + { + &C_icon_validate_14, + "Accept risk and", + "review transaction", + }); +UX_STEP_CB( + ux_blind_signing_reject_step, + pb, + report_finalize_error(), { &C_icon_crossmark, - "Blind signing must be", - "enabled in Settings", + "Reject", }); #endif // clang-format on -UX_FLOW(ux_warning_contract_data_flow, &ux_warning_contract_data_step); +UX_FLOW(ux_blind_signing_flow, + &ux_blind_signing_warning_step, + &ux_blind_signing_text1_step, + &ux_blind_signing_text2_step, + &ux_blind_signing_link_step, + &ux_blind_signing_accept_step, + &ux_blind_signing_reject_step); diff --git a/src_bagl/ui_flow.h b/src_bagl/ui_flow.h index ae2590d..e3d4270 100644 --- a/src_bagl/ui_flow.h +++ b/src_bagl/ui_flow.h @@ -8,7 +8,7 @@ extern const ux_flow_step_t* const ux_idle_flow[]; -extern const ux_flow_step_t* const ux_warning_contract_data_flow[]; +extern const ux_flow_step_t* const ux_blind_signing_flow[]; extern const ux_flow_step_t* const ux_settings_flow[]; diff --git a/src_bagl/ui_flow_signTx.c b/src_bagl/ui_flow_signTx.c index c0ecfcc..d32afe7 100644 --- a/src_bagl/ui_flow_signTx.c +++ b/src_bagl/ui_flow_signTx.c @@ -199,12 +199,12 @@ UX_STEP_NOCB( .text = strings.common.nonce, }); -UX_STEP_NOCB(ux_approval_blind_signing_warning_step, +UX_STEP_NOCB(ux_approval_blind_signing_reminder_step, pbb, { &C_icon_warning, - "Blind", - "Signing", + "You accepted", + "the risks", }); // clang-format on @@ -214,10 +214,6 @@ void ux_approve_tx(bool fromPlugin) { int step = 0; ux_approval_tx_flow[step++] = &ux_approval_review_step; - if (!fromPlugin && tmpContent.txContent.dataPresent && !N_storage.contractDetails) { - ux_approval_tx_flow[step++] = &ux_approval_blind_signing_warning_step; - } - if (fromPlugin) { // Add the special dynamic display logic ux_approval_tx_flow[step++] = &ux_plugin_approval_id_step; @@ -260,6 +256,9 @@ void ux_approve_tx(bool fromPlugin) { } ux_approval_tx_flow[step++] = &ux_approval_fees_step; + if (!fromPlugin && tmpContent.txContent.dataPresent && !N_storage.contractDetails) { + ux_approval_tx_flow[step++] = &ux_approval_blind_signing_reminder_step; + } ux_approval_tx_flow[step++] = &ux_approval_accept_step; ux_approval_tx_flow[step++] = &ux_approval_reject_step; ux_approval_tx_flow[step++] = FLOW_END_STEP; diff --git a/src_features/signTx/cmd_signTx.c b/src_features/signTx/cmd_signTx.c index 8a84538..4da3a76 100644 --- a/src_features/signTx/cmd_signTx.c +++ b/src_features/signTx/cmd_signTx.c @@ -88,7 +88,7 @@ void handleSign(uint8_t p1, } if (txResult == USTREAM_FINISHED) { - finalizeParsing(false); + finalizeParsing(); } *flags |= IO_ASYNCH_REPLY; diff --git a/src_features/signTx/feature_signTx.h b/src_features/signTx/feature_signTx.h index 84d2cd5..a62514a 100644 --- a/src_features/signTx/feature_signTx.h +++ b/src_features/signTx/feature_signTx.h @@ -11,9 +11,11 @@ typedef enum { } plugin_ui_state_t; customStatus_e customProcessor(txContext_t *context); -void finalizeParsing(bool direct); +void finalizeParsing(); void prepareFeeDisplay(); void prepareNetworkDisplay(); void ux_approve_tx(bool fromPlugin); +void report_finalize_error(void); +void start_signature_flow(void); #endif // _SIGN_TX_H_ diff --git a/src_features/signTx/logic_signTx.c b/src_features/signTx/logic_signTx.c index 3e8238f..f9f8a8c 100644 --- a/src_features/signTx/logic_signTx.c +++ b/src_features/signTx/logic_signTx.c @@ -14,6 +14,8 @@ #define ERR_SILENT_MODE_CHECK_FAILED 0x6001 +static bool g_use_standard_ui; + static uint32_t splitBinaryParameterPart(char *result, size_t result_size, uint8_t *parameter) { uint32_t i; for (i = 0; i < 8; i++) { @@ -172,14 +174,10 @@ customStatus_e customProcessor(txContext_t *context) { return CUSTOM_NOT_HANDLED; } -void reportFinalizeError(bool direct) { +void report_finalize_error(void) { reset_app_context(); - if (direct) { - THROW(0x6A80); - } else { - io_seproxyhal_send_status(0x6A80); - ui_idle(); - } + io_seproxyhal_send_status(APDU_RESPONSE_INVALID_DATA); + ui_idle(); } static void address_to_string(uint8_t *in, @@ -312,7 +310,7 @@ static int strcasecmp_workaround(const char *str1, const char *str2) { return 0; } -__attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool *use_standard_UI) { +__attribute__((noinline)) static bool finalize_parsing_helper(void) { char displayBuffer[50]; uint8_t decimals = WEI_TO_ETHER; uint64_t chain_id = get_tx_chain_id(); @@ -327,10 +325,8 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool if (chainConfig->chainId != id) { PRINTF("Invalid chainID %u expected %u\n", id, chainConfig->chainId); reset_app_context(); - reportFinalizeError(direct); - if (!direct) { - return false; - } + report_finalize_error(); + return false; } } // Store the hash @@ -358,10 +354,8 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool if (!eth_plugin_call(ETH_PLUGIN_FINALIZE, (void *) &pluginFinalize)) { PRINTF("Plugin finalize call failed\n"); - reportFinalizeError(direct); - if (!direct) { - return false; - } + report_finalize_error(); + return false; } // Lookup tokens if requested ethPluginProvideInfo_t pluginProvideInfo; @@ -384,10 +378,8 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool if (eth_plugin_call(ETH_PLUGIN_PROVIDE_INFO, (void *) &pluginProvideInfo) <= ETH_PLUGIN_RESULT_UNSUCCESSFUL) { PRINTF("Plugin provide token call failed\n"); - reportFinalizeError(direct); - if (!direct) { - return false; - } + report_finalize_error(); + return false; } pluginFinalize.result = pluginProvideInfo.result; } @@ -396,7 +388,7 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool switch (pluginFinalize.uiType) { case ETH_UI_TYPE_GENERIC: // Use the dedicated ETH plugin UI - *use_standard_UI = false; + g_use_standard_ui = false; tmpContent.txContent.dataPresent = false; // Add the number of screens + the number of additional screens to get the total // number of screens needed. @@ -405,14 +397,12 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool break; case ETH_UI_TYPE_AMOUNT_ADDRESS: // Use the standard ETH UI as this plugin uses the amount/address UI - *use_standard_UI = true; + g_use_standard_ui = true; tmpContent.txContent.dataPresent = false; if ((pluginFinalize.amount == NULL) || (pluginFinalize.address == NULL)) { PRINTF("Incorrect amount/address set by plugin\n"); - reportFinalizeError(direct); - if (!direct) { - return false; - } + report_finalize_error(); + return false; } memmove(tmpContent.txContent.value.value, pluginFinalize.amount, 32); tmpContent.txContent.value.length = 32; @@ -425,10 +415,8 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool break; default: PRINTF("ui type %d not supported\n", pluginFinalize.uiType); - reportFinalizeError(direct); - if (!direct) { - return false; - } + report_finalize_error(); + return false; } } } @@ -443,21 +431,13 @@ __attribute__((noinline)) static bool finalize_parsing_helper(bool direct, bool } // User has just validated a swap but ETH received apdus about a non standard plugin / contract - if (G_called_from_swap && !*use_standard_UI) { + if (G_called_from_swap && !g_use_standard_ui) { PRINTF("ERR_SILENT_MODE_CHECK_FAILED, G_called_from_swap\n"); THROW(ERR_SILENT_MODE_CHECK_FAILED); } - if (tmpContent.txContent.dataPresent) { - reportFinalizeError(direct); - ui_warning_contract_data(); - if (!direct) { - return false; - } - } - // Prepare destination address and amount to display - if (*use_standard_UI) { + if (g_use_standard_ui) { // Format the address in a temporary buffer, if in swap case compare it with validated // address, else commit it address_to_string(tmpContent.txContent.destination, @@ -536,23 +516,32 @@ end: return false; } -void finalizeParsing(bool direct) { - bool use_standard_UI = true; +void start_signature_flow(void) { + if (g_use_standard_ui) { + ux_approve_tx(false); + } else { + dataContext.tokenContext.pluginUiState = PLUGIN_UI_OUTSIDE; + dataContext.tokenContext.pluginUiCurrentItem = 0; + ux_approve_tx(true); + } +} - if (!finalize_parsing_helper(direct, &use_standard_UI)) { +void finalizeParsing(void) { + g_use_standard_ui = true; + + if (!finalize_parsing_helper()) { return; } // If called from swap, the user has already validated a standard transaction // And we have already checked the fields of this transaction above - if (G_called_from_swap && use_standard_UI) { + if (G_called_from_swap && g_use_standard_ui) { io_seproxyhal_touch_tx_ok(NULL); } else { - if (use_standard_UI) { - ux_approve_tx(false); + // If blind-signing detected, start the warning flow beforehand + if (tmpContent.txContent.dataPresent) { + ui_warning_contract_data(); } else { - dataContext.tokenContext.pluginUiState = PLUGIN_UI_OUTSIDE; - dataContext.tokenContext.pluginUiCurrentItem = 0; - ux_approve_tx(true); + start_signature_flow(); } } } diff --git a/src_features/signTx/ui_common_signTx.c b/src_features/signTx/ui_common_signTx.c index 9041c70..4b567db 100644 --- a/src_features/signTx/ui_common_signTx.c +++ b/src_features/signTx/ui_common_signTx.c @@ -4,6 +4,7 @@ #include "common_utils.h" #include "common_ui.h" #include "handle_swap_sign_transaction.h" +#include "feature_signTx.h" unsigned int io_seproxyhal_touch_tx_ok(__attribute__((unused)) const bagl_element_t *e) { uint32_t info = 0; @@ -109,7 +110,7 @@ unsigned int io_seproxyhal_touch_data_ok(__attribute__((unused)) const bagl_elem } if (txResult == USTREAM_FINISHED) { - finalizeParsing(false); + finalizeParsing(); } return 0;