Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/features/set_external_plugin/cmd_set_external_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,20 @@ uint16_t handle_set_external_plugin(const uint8_t *workBuffer, uint8_t dataLengt
workBuffer += ADDRESS_LENGTH;
memmove(dataContext.tokenContext.methodSelector, workBuffer, SELECTOR_SIZE);
pluginType = PLUGIN_TYPE_EXTERNAL;
// External-plugin registration is intentionally chain-unbound: the signed
// payload here is [name_len | name | address | selector] and contains no
// chain_id field, unlike handle_set_plugin which does carry one. Marking
// the binding as PLUGIN_CHAIN_ID_ANY is required so the deferred chain
// check in finalize_parsing_helper() lets the registration through.
//
// Closing the cross-chain replay surface for external plugins (Cerberus
// CWE-345 follow-up) requires extending the signed payload format on
// both the device and the metadata-signing side, and is intentionally
// out of scope of this fix. Tracked separately, do not patch in place:
// naively widening payload_size here would either reject every existing
// signature (the extra bytes are not yet sent) or accept attacker-
// controlled bytes outside the signed range.
dataContext.tokenContext.pluginChainId = PLUGIN_CHAIN_ID_ANY;

return SWO_SUCCESS;
}
5 changes: 5 additions & 0 deletions src/features/set_plugin/cmd_set_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,11 @@ uint16_t handle_set_plugin(const uint8_t *workBuffer, uint8_t dataLength) {
UNSUPPORTED_CHAIN_ID_MSG(chain_id);
return SWO_INCORRECT_DATA;
}
// Bind the registration to the signed chain. Without this, a host could
// load a valid signed plugin registration for one EVM chain and then have
// the device activate the plugin UI for a transaction on a different
// chain where the same address/selector means something else.
tokenContext->pluginChainId = chain_id;
offset += CHAIN_ID_SIZE;

keyId = workBuffer[offset];
Expand Down
15 changes: 15 additions & 0 deletions src/features/sign_authorization_eip7702/commands_7702.c
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,28 @@ uint16_t handle_sign_eip7702_authorization(uint8_t p1,
uint8_t dataLength) {
g_7702_sw = SWO_PARAMETER_ERROR_NO_INFO;
if (p1 == P1_FIRST_CHUNK) {
// Lock the EIP-7702 authorization flow against being restarted while
// another signing/review is in progress. Without this guard, a hostile
// host could overwrite tmpCtx.authSigningContext7702.bip32 during a
// pending review and trick the user into signing with a path other
// than the one displayed on screen.
if (appState != APP_STATE_IDLE) {
PRINTF("Cannot start an EIP-7702 authorization while another flow is active\n");
return SWO_COMMAND_NOT_ALLOWED;
}
appState = APP_STATE_SIGNING_EIP7702;
if ((dataBuffer =
parseBip32(dataBuffer, &dataLength, &tmpCtx.authSigningContext7702.bip32)) ==
NULL) {
reset_app_context();
return SWO_INCORRECT_DATA;
}
} else if (appState != APP_STATE_SIGNING_EIP7702) {
PRINTF("EIP-7702 continuation chunk without an active authorization session\n");
return SWO_COMMAND_NOT_ALLOWED;
}
if (!tlv_from_apdu(p1 == P1_FIRST_CHUNK, dataLength, dataBuffer, &handle_auth7702_tlv)) {
reset_app_context();
return g_7702_sw;
}
return SWO_NO_RESPONSE;
Expand Down
4 changes: 3 additions & 1 deletion src/features/sign_authorization_eip7702/ui_common_7702.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ unsigned int auth_7702_ok_cb(void) {
} else {
G_io_tx_buffer[0] = 0;
}
return io_seproxyhal_send_status(SWO_SUCCESS, ECDSA_SIGNATURE_LENGTH, false, true);
// Reset to release the APP_STATE_SIGNING_EIP7702 lock and clear the
// signing context so a new authorization can start from a clean slate.
return io_seproxyhal_send_status(SWO_SUCCESS, ECDSA_SIGNATURE_LENGTH, true, true);
}

unsigned int auth_7702_cancel_cb(void) {
Expand Down
14 changes: 14 additions & 0 deletions src/features/sign_tx/logic_sign_tx.c
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,20 @@ __attribute__((noinline)) static uint16_t finalize_parsing_helper(const txContex
goto end;
}
PRINTF("FROM address displayed: %s\n", strings.common.fromAddress);
// Enforce chain binding for plugin registrations now that the full tx has
// been parsed and get_tx_chain_id() is reliable (LEGACY transactions only
// expose chain_id through the V field, which is parsed after the plugin
// init triggered on the data field).
if ((dataContext.tokenContext.pluginStatus >= ETH_PLUGIN_RESULT_SUCCESSFUL) &&
(dataContext.tokenContext.pluginChainId != PLUGIN_CHAIN_ID_ANY) &&
(dataContext.tokenContext.pluginChainId != chain_id)) {
PRINTF("Plugin registered for chain %llu but tx is on chain %llu\n",
dataContext.tokenContext.pluginChainId,
chain_id);
report_finalize_error();
error = SWO_NO_RESPONSE;
goto end;
}
// Finalize the plugin handling
if (dataContext.tokenContext.pluginStatus >= ETH_PLUGIN_RESULT_SUCCESSFUL) {
eth_plugin_prepare_finalize(&pluginFinalize);
Expand Down
24 changes: 21 additions & 3 deletions src/nbgl/ui_home.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,20 @@ nbgl_warning_t warning;
// Tagline format for plugins
#define FORMAT_PLUGIN "This app enables clear\nsigning transactions for\nthe %s dApp."

// Maximum caller-provided plugin name length we accept for the tagline. 64 is
// generous compared to real Ledger plugin app names (typically < 20 chars) and
// the fuzz harnesses' NAME_LENGTH=32, while staying well below the byte that
// would let the total line length wrap. Any value below ~196 closes the
// CWE-120 overflow.
#define MAX_PLUGIN_NAME_LEN 64

// Catch any future change to FORMAT_PLUGIN or MAX_PLUGIN_NAME_LEN that would
// re-introduce a path where the tagline allocation length wraps. sizeof
// includes the NUL terminator; 256 keeps a comfortable margin below the 8-bit
// boundary that used to wrap the original uint8_t accumulator.
_Static_assert(sizeof(FORMAT_PLUGIN) + MAX_PLUGIN_NAME_LEN < 256,
"Plugin tagline buffer math exceeds the historical 8-bit bound ");

enum {
TRANSACTION_CHECKS_TOKEN = FIRST_USER_TOKEN,
BLIND_SIGNING_TOKEN,
Expand Down Expand Up @@ -211,14 +225,18 @@ static void prepare_and_display_home(const char *appname, const char *tagline, u
*/
static void get_appname_and_tagline(const char **appname, const char **tagline) {
uint64_t mainnet_chain_id;
uint8_t line_len = 1; // Initialize lengths to 1 for '\0' character

if (caller_app) {
*appname = caller_app->name;

if (caller_app->type == CALLER_TYPE_PLUGIN) {
line_len += strlen(FORMAT_PLUGIN);
line_len += strlen(caller_app->name);
size_t name_len = strnlen(caller_app->name, MAX_PLUGIN_NAME_LEN + 1);
if (name_len > MAX_PLUGIN_NAME_LEN) {
PRINTF("Plugin caller_app->name exceeds %u bytes; tagline omitted\n",
(unsigned) MAX_PLUGIN_NAME_LEN);
return;
}
size_t line_len = 1 + strlen(FORMAT_PLUGIN) + name_len;
// Allocate the buffer - will never be deallocated...
if (APP_MEM_CALLOC((void **) &g_tag_line, line_len) == true) {
snprintf(g_tag_line, line_len, FORMAT_PLUGIN, *appname);
Expand Down
75 changes: 69 additions & 6 deletions src/plugins/eip7002/eip7002_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,48 @@ static void eip7002_plugin_provider_parameter(ethPluginProvideParameter_t *param
}
}

// EIP-7002 charges a dynamic per-request fee paid in native value. In normal
// operation this fee is in the wei-to-gwei range — showing a "Tx value: 1 wei"
// screen on every legitimate request is noisy without informing the user.
// Hide the screen as long as the value stays below this threshold; anything
// above is worth surfacing because it dwarfs the protocol fee. The attacker
// budget for a hidden value is therefore capped at TX_VALUE_MIN_DISPLAY_WEI,
// i.e. dust ($0.000004 at $4000/ETH), instead of the unbounded original CVE.
#define TX_VALUE_MIN_DISPLAY_WEI 1000000000ULL // 1 gwei

// Whether the transaction carries a native value worth surfacing to the user.
// NULL-safe so callers can pass param->txContent directly from either the
// ETH_PLUGIN_FINALIZE or ETH_PLUGIN_QUERY_CONTRACT_UI message structs.
static bool has_tx_value(const txContent_t *txContent) {
if ((txContent == NULL) || (txContent->value.length == 0)) {
return false;
}
if (txContent->value.length > sizeof(uint64_t)) {
// > 2^64 wei is unambiguously larger than the threshold.
return true;
}
uint64_t val = 0;
for (uint8_t i = 0; i < txContent->value.length; ++i) {
val = (val << 8) | txContent->value.value[i];
}
return val > TX_VALUE_MIN_DISPLAY_WEI;
}

static void eip7002_plugin_finalize(ethPluginFinalize_t *param) {
eip7002_context_t *context = (eip7002_context_t *) param->pluginContext;

param->uiType = ETH_UI_TYPE_GENERIC;
param->numScreens = is_zeroes_buffer(context->raw_amount, sizeof(context->raw_amount)) ? 1 : 2;
// Validator screen is always shown. The native tx.value is shown only
// when it exceeds the dust threshold defined by has_tx_value, so a
// hostile dApp cannot smuggle a meaningful ETH/native value into a
// staking-style request that otherwise only renders calldata.
param->numScreens = 1;
if (has_tx_value(param->txContent)) {
param->numScreens++;
}
if (!is_zeroes_buffer(context->raw_amount, sizeof(context->raw_amount))) {
param->numScreens++;
}
param->result = (context->received == sizeof(context->withdrawal_request))
? ETH_PLUGIN_RESULT_OK
: ETH_PLUGIN_RESULT_ERROR;
Expand All @@ -88,9 +125,23 @@ static void eip7002_plugin_query_contract_ui(ethQueryContractUI_t *param) {
eip7002_context_t *context = (eip7002_context_t *) param->pluginContext;
uint64_t chain_id = get_tx_chain_id();
const char *ticker = get_displayable_ticker(&chain_id, g_chain_config, true);
// Map a screen index to a logical screen kind based on which optional
// screens are present for this transaction.
bool show_tx_value = has_tx_value(param->txContent);
bool show_request_amount = !is_zeroes_buffer(context->raw_amount, sizeof(context->raw_amount));
uint8_t idx = param->screenIndex;
enum { S_VALIDATOR, S_TX_VALUE, S_REQUEST_AMOUNT, S_UNKNOWN } screen = S_UNKNOWN;

if (idx == 0) {
screen = S_VALIDATOR;
} else if (show_tx_value && idx == 1) {
screen = S_TX_VALUE;
} else if (show_request_amount && idx == (show_tx_value ? 2 : 1)) {
screen = S_REQUEST_AMOUNT;
}

switch (param->screenIndex) {
case 0:
switch (screen) {
case S_VALIDATOR:
if (param->msgLength < 2) {
return;
}
Expand All @@ -101,7 +152,19 @@ static void eip7002_plugin_query_contract_ui(ethQueryContractUI_t *param) {
&param->msg[2],
param->msgLength - 2);
break;
case 1:
case S_TX_VALUE:
strlcpy(param->title, "Tx value", param->titleLength);
if (!amountToString(param->txContent->value.value,
param->txContent->value.length,
WEI_TO_ETHER,
ticker,
param->msg,
param->msgLength)) {
param->result = ETH_PLUGIN_RESULT_ERROR;
return;
}
break;
case S_REQUEST_AMOUNT:
strlcpy(param->title, "Amount", param->titleLength);
if (!amountToString(context->raw_amount,
sizeof(context->raw_amount),
Expand All @@ -110,10 +173,10 @@ static void eip7002_plugin_query_contract_ui(ethQueryContractUI_t *param) {
param->msg,
param->msgLength)) {
param->result = ETH_PLUGIN_RESULT_ERROR;
break;
return;
}
break;
default:
case S_UNKNOWN:
break;
}
param->result = ETH_PLUGIN_RESULT_OK;
Expand Down
115 changes: 90 additions & 25 deletions src/plugins/eip7251/eip7251_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,11 +64,49 @@ static void eip7251_plugin_provider_parameter(ethPluginProvideParameter_t *param
}
}

// EIP-7251 charges a dynamic per-request fee paid in native value. In normal
// operation this fee is in the wei-to-gwei range — showing a "Tx value: 1 wei"
// screen on every legitimate request is noisy without informing the user.
// Hide the screen as long as the value stays below this threshold; anything
// above is worth surfacing because it dwarfs the protocol fee. The attacker
// budget for a hidden value is therefore capped at TX_VALUE_MIN_DISPLAY_WEI,
// i.e. dust ($0.000004 at $4000/ETH), instead of the unbounded original CVE.
#define TX_VALUE_MIN_DISPLAY_WEI 1000000000ULL // 1 gwei

// Whether the transaction carries a native value worth surfacing to the user.
// NULL-safe so callers can pass param->txContent directly from either the
// ETH_PLUGIN_FINALIZE or ETH_PLUGIN_QUERY_CONTRACT_UI message structs.
static bool has_tx_value(const txContent_t *txContent) {
if ((txContent == NULL) || (txContent->value.length == 0)) {
return false;
}
if (txContent->value.length > sizeof(uint64_t)) {
// > 2^64 wei is unambiguously larger than the threshold.
return true;
}
uint64_t val = 0;
for (uint8_t i = 0; i < txContent->value.length; ++i) {
val = (val << 8) | txContent->value.value[i];
}
return val > TX_VALUE_MIN_DISPLAY_WEI;
}

static void eip7251_plugin_finalize(ethPluginFinalize_t *param) {
eip7251_context_t *context = (eip7251_context_t *) param->pluginContext;

param->uiType = ETH_UI_TYPE_GENERIC;
param->numScreens = target_equals_source(context) ? 1 : 2;
// Source validator is always shown. Target is shown when distinct. The
// native tx.value is shown only when it exceeds the dust threshold
// defined by has_tx_value, so a hostile dApp cannot smuggle a meaningful
// ETH/native value into a consolidation request that otherwise only
// renders validator pubkeys.
param->numScreens = 1;
if (!target_equals_source(context)) {
param->numScreens++;
}
if (has_tx_value(param->txContent)) {
param->numScreens++;
}
param->result = (context->received == sizeof(context->consolidation_request))
? ETH_PLUGIN_RESULT_OK
: ETH_PLUGIN_RESULT_ERROR;
Expand All @@ -86,33 +124,60 @@ static void eip7251_plugin_query_contract_id(ethQueryContractID_t *param) {

static void eip7251_plugin_query_contract_ui(ethQueryContractUI_t *param) {
eip7251_context_t *context = (eip7251_context_t *) param->pluginContext;
// Map a screen index to a logical screen kind based on which optional
// screens are present for this transaction.
bool show_target = !target_equals_source(context);
bool show_tx_value = has_tx_value(param->txContent);
uint8_t idx = param->screenIndex;
enum { S_SOURCE, S_TARGET, S_TX_VALUE, S_UNKNOWN } screen = S_UNKNOWN;

if (idx == 0) {
screen = S_SOURCE;
} else if (show_target && idx == 1) {
screen = S_TARGET;
} else if (show_tx_value && idx == (show_target ? 2 : 1)) {
screen = S_TX_VALUE;
}

if (param->msgLength >= 2) {
memcpy(param->msg, "0x", 2);
switch (param->screenIndex) {
case 0:
if (target_equals_source(context)) {
strlcpy(param->title, "Validator", param->titleLength);
} else {
strlcpy(param->title, "From validator", param->titleLength);
}
format_hex(context->source_pubkey,
sizeof(context->source_pubkey),
&param->msg[2],
param->msgLength - 2);
break;
case 1:
strlcpy(param->title, "To validator", param->titleLength);
format_hex(context->target_pubkey,
sizeof(context->target_pubkey),
&param->msg[2],
param->msgLength - 2);
break;
default:
break;
if (param->msgLength < 2) {
return;
}
switch (screen) {
case S_SOURCE:
memcpy(param->msg, "0x", 2);
strlcpy(param->title, show_target ? "From validator" : "Validator", param->titleLength);
format_hex(context->source_pubkey,
sizeof(context->source_pubkey),
&param->msg[2],
param->msgLength - 2);
break;
case S_TARGET:
memcpy(param->msg, "0x", 2);
strlcpy(param->title, "To validator", param->titleLength);
format_hex(context->target_pubkey,
sizeof(context->target_pubkey),
&param->msg[2],
param->msgLength - 2);
break;
case S_TX_VALUE: {
uint64_t chain_id = get_tx_chain_id();
const char *ticker = get_displayable_ticker(&chain_id, g_chain_config, true);
strlcpy(param->title, "Tx value", param->titleLength);
if (!amountToString(param->txContent->value.value,
param->txContent->value.length,
WEI_TO_ETHER,
ticker,
param->msg,
param->msgLength)) {
param->result = ETH_PLUGIN_RESULT_ERROR;
return;
}
break;
}
param->result = ETH_PLUGIN_RESULT_OK;
case S_UNKNOWN:
return;
}
param->result = ETH_PLUGIN_RESULT_OK;
}

void eip7251_plugin_call(eth_plugin_msg_t msg, void *param) {
Expand Down
8 changes: 7 additions & 1 deletion src/plugins/erc1155/erc1155_plugin.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,13 @@ void handle_finalize_1155(ethPluginFinalize_t *msg) {
msg->numScreens = 5;
break;
case SAFE_BATCH_TRANSFER:
msg->numScreens = 4;
// To, Collection Name, NFT Address, Total Quantity
// + 2 screens per displayed pair (ID + Quantity)
// + 1 warning screen if truncated.
msg->numScreens = 4 + 2 * context->batch_displayed;
if (context->batch_truncated) {
msg->numScreens += 1;
}
break;
case SET_APPROVAL_FOR_ALL:
msg->numScreens = 3;
Expand Down
Loading
Loading