Skip to content

Commit 50867ce

Browse files
fix: Cross-chain plugin registration reuse
handle_set_plugin() parsed the signed chain_id only to call app_compatible_with_chain_id() and then discarded it. The stored tokenContext kept the plugin name, contract address, and selector but never the chain the registration was signed for. eth_plugin_perform_ init_default() therefore authorized the plugin solely on address + selector match, which let a host preload a valid signed registration for chain A and then route a transaction on chain B through the same plugin UI, masking attacker-controlled calldata behind a familiar review flow. Store the signed chain_id alongside the registration and refuse to activate the plugin when the transaction's chain_id differs. set_external_plugin keeps its chain-unbound semantics (its signed payload contains no chain_id) and is marked explicitly via PLUGIN_CHAIN_ID_ANY; cross-chain protection there requires a protocol change and is left out of scope. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 5331b80 commit 50867ce

5 files changed

Lines changed: 51 additions & 0 deletions

File tree

src/features/set_external_plugin/cmd_set_external_plugin.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ uint16_t handle_set_external_plugin(const uint8_t *workBuffer, uint8_t dataLengt
7373
workBuffer += ADDRESS_LENGTH;
7474
memmove(dataContext.tokenContext.methodSelector, workBuffer, SELECTOR_SIZE);
7575
pluginType = PLUGIN_TYPE_EXTERNAL;
76+
// External-plugin registration is not bound to a chain (no chain_id in the
77+
// signed payload), so we mark it ANY here. Cross-chain replay protection
78+
// for external plugins requires extending the signed payload to include
79+
// chain_id, which is a protocol change outside the scope of this fix.
80+
dataContext.tokenContext.pluginChainId = PLUGIN_CHAIN_ID_ANY;
7681

7782
return SWO_SUCCESS;
7883
}

src/features/set_plugin/cmd_set_plugin.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,11 @@ uint16_t handle_set_plugin(const uint8_t *workBuffer, uint8_t dataLength) {
145145
UNSUPPORTED_CHAIN_ID_MSG(chain_id);
146146
return SWO_INCORRECT_DATA;
147147
}
148+
// Bind the registration to the signed chain. Without this, a host could
149+
// load a valid signed plugin registration for one EVM chain and then have
150+
// the device activate the plugin UI for a transaction on a different
151+
// chain where the same address/selector means something else.
152+
tokenContext->pluginChainId = chain_id;
148153
offset += CHAIN_ID_SIZE;
149154

150155
keyId = workBuffer[offset];

src/features/sign_tx/logic_sign_tx.c

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,19 @@ __attribute__((noinline)) static uint16_t finalize_parsing_helper(const txContex
318318
goto end;
319319
}
320320
PRINTF("FROM address displayed: %s\n", strings.common.fromAddress);
321+
// Enforce chain binding for plugin registrations now that the full tx has
322+
// been parsed and get_tx_chain_id() is reliable (LEGACY transactions only
323+
// expose chain_id through the V field, which is parsed after the plugin
324+
// init triggered on the data field).
325+
if ((dataContext.tokenContext.pluginStatus >= ETH_PLUGIN_RESULT_SUCCESSFUL) &&
326+
(dataContext.tokenContext.pluginChainId != PLUGIN_CHAIN_ID_ANY) &&
327+
(dataContext.tokenContext.pluginChainId != chain_id)) {
328+
PRINTF("Plugin registered for chain %llu but tx is on chain %llu\n",
329+
dataContext.tokenContext.pluginChainId,
330+
chain_id);
331+
report_finalize_error();
332+
return SWO_NO_RESPONSE;
333+
}
321334
// Finalize the plugin handling
322335
if (dataContext.tokenContext.pluginStatus >= ETH_PLUGIN_RESULT_SUCCESSFUL) {
323336
eth_plugin_prepare_finalize(&pluginFinalize);

src/plugins/eth_plugin_handler.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,24 @@ void eth_plugin_prepare_query_contract_ui(ethQueryContractUI_t *query_contract_u
9898

9999
static void eth_plugin_perform_init_default(uint8_t *contract_address,
100100
ethPluginInitContract_t *init) {
101+
// Refuse to activate a plugin registration that was signed for a different
102+
// chain than the one we are about to sign on. Skipped when:
103+
// * the registration is chain-unbound (e.g., set_external_plugin payload
104+
// carries no chain_id field, marked as PLUGIN_CHAIN_ID_ANY);
105+
// * we cannot yet resolve the transaction chain_id (LEGACY transactions
106+
// only expose it through the V field, which is parsed after the data
107+
// field that triggers this init). In that case the check is deferred
108+
// to finalize_parsing_helper(), which runs once V has been parsed.
109+
if (dataContext.tokenContext.pluginChainId != PLUGIN_CHAIN_ID_ANY) {
110+
uint64_t tx_chain_id = get_tx_chain_id();
111+
if ((tx_chain_id != 0) && (tx_chain_id != dataContext.tokenContext.pluginChainId)) {
112+
PRINTF("Plugin registered for chain %llu but tx is on chain %llu\n",
113+
dataContext.tokenContext.pluginChainId,
114+
tx_chain_id);
115+
dataContext.tokenContext.pluginStatus = ETH_PLUGIN_RESULT_UNAVAILABLE;
116+
return;
117+
}
118+
}
101119
// check if the registered external plugin matches the TX contract address / selector
102120
if (memcmp(contract_address,
103121
dataContext.tokenContext.contractAddress,

src/shared_context.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,11 @@ typedef struct internalStorage_t {
5050
bool initialized;
5151
} internalStorage_t;
5252

53+
// Sentinel for tokenContext_t.pluginChainId meaning "registration is not bound
54+
// to a specific chain" (used by paths whose signed metadata does not carry a
55+
// chain_id). All real EVM chain IDs are > 0.
56+
#define PLUGIN_CHAIN_ID_ANY 0
57+
5358
typedef struct tokenContext_t {
5459
char pluginName[PLUGIN_ID_LENGTH];
5560

@@ -76,6 +81,11 @@ typedef struct tokenContext_t {
7681

7782
uint8_t pluginStatus;
7883

84+
// Chain ID the plugin registration was issued for. Populated from the
85+
// signed SET_PLUGIN payload so we can refuse to activate the plugin on a
86+
// transaction whose chain_id differs.
87+
uint64_t pluginChainId;
88+
7989
} tokenContext_t;
8090

8191
_Static_assert((offsetof(tokenContext_t, pluginContext) % 4) == 0, "Plugin context not aligned");

0 commit comments

Comments
 (0)