Skip to content

Commit 2a6dfb7

Browse files
fix: ERC1155 batch-transfer UI omits token IDs and per-ID quantities
The ERC1155 safeBatchTransferFrom (0x2eb2c2d6) review path explicitly skipped copying token IDs (`// don't copy anything since we won't display it`) and summed every value into a single context->value. The device then only rendered "<total> from <count> NFT IDs", which let a malicious dApp slip a high-value token ID into a batch of innocuous ones without the user being able to detect it on-screen. Capture the first ERC1155_BATCH_DISPLAY_MAX (id, value) pairs into the plugin context, render each pair on its own pair of screens, and add an explicit "WARNING — Only first N of M IDs shown" screen whenever the batch is larger than what we can surface. The aggregate Total Quantity screen is preserved. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 6be2b81 commit 2a6dfb7

131 files changed

Lines changed: 88 additions & 10 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/plugins/erc1155/erc1155_plugin.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,13 @@ void handle_finalize_1155(ethPluginFinalize_t *msg) {
5959
msg->numScreens = 5;
6060
break;
6161
case SAFE_BATCH_TRANSFER:
62-
msg->numScreens = 4;
62+
// To, Collection Name, NFT Address, Total Quantity
63+
// + 2 screens per displayed pair (ID + Quantity)
64+
// + 1 warning screen if truncated.
65+
msg->numScreens = 4 + 2 * context->batch_displayed;
66+
if (context->batch_truncated) {
67+
msg->numScreens += 1;
68+
}
6369
break;
6470
case SET_APPROVAL_FOR_ALL:
6571
msg->numScreens = 3;

src/plugins/erc1155/erc1155_plugin.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@
88

99
// Internal plugin for EIP 1155: https://eips.ethereum.org/EIPS/eip-1155
1010

11+
// Maximum number of (id, value) pairs surfaced individually during a
12+
// safeBatchTransferFrom review. Anything beyond this is reported via the
13+
// aggregate quantity screen plus a truncation warning so the user is told
14+
// the on-device view is incomplete.
15+
#define ERC1155_BATCH_DISPLAY_MAX 3
16+
1117
typedef struct erc1155_context_t {
1218
uint8_t address[ADDRESS_LENGTH];
1319
uint8_t tokenId[INT256_LENGTH];
@@ -22,6 +28,14 @@ typedef struct erc1155_context_t {
2228
bool approved;
2329
uint8_t next_param;
2430
uint8_t selectorIndex;
31+
32+
// SAFE_BATCH_TRANSFER review data. Without these, the UI only displayed
33+
// an aggregate "<total> from <count> NFT IDs" line and gave the user no
34+
// way to detect a high-value token ID hidden among innocuous ones.
35+
uint8_t batch_ids[ERC1155_BATCH_DISPLAY_MAX][INT256_LENGTH];
36+
uint8_t batch_values[ERC1155_BATCH_DISPLAY_MAX][INT256_LENGTH];
37+
uint8_t batch_displayed;
38+
bool batch_truncated;
2539
} erc1155_context_t;
2640

2741
void handle_provide_parameter_1155(ethPluginProvideParameter_t *parameters);

src/plugins/erc1155/erc1155_provide_parameters.c

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,20 @@ static void handle_batch_transfer(ethPluginProvideParameter_t *msg, erc1155_cont
6363
}
6464
context->ids_array_len =
6565
U2BE(msg->parameter, PARAMETER_LENGTH - sizeof(context->ids_array_len));
66+
context->batch_displayed = (context->ids_array_len > ERC1155_BATCH_DISPLAY_MAX)
67+
? ERC1155_BATCH_DISPLAY_MAX
68+
: (uint8_t) context->ids_array_len;
69+
context->batch_truncated = context->ids_array_len > ERC1155_BATCH_DISPLAY_MAX;
6670
context->next_param = TOKEN_ID;
6771
// set to zero for next step
6872
context->array_index = 0;
6973
break;
7074
case TOKEN_ID:
71-
// don't copy anything since we won't display it
75+
// Surface the first batch entries to the user so a malicious
76+
// batch cannot hide a high-value token ID behind innocuous ones.
77+
if (context->array_index < ERC1155_BATCH_DISPLAY_MAX) {
78+
memcpy(context->batch_ids[context->array_index], msg->parameter, INT256_LENGTH);
79+
}
7280
if (--context->ids_array_len == 0) {
7381
context->next_param = VALUE_LENGTH;
7482
}
@@ -98,6 +106,12 @@ static void handle_batch_transfer(ethPluginProvideParameter_t *msg, erc1155_cont
98106
case VALUE:
99107
// put it temporarily in token id since we don't use it in batch transfer
100108
copy_parameter(context->tokenId, msg->parameter, sizeof(context->value));
109+
// Keep the first per-id values so they pair up with batch_ids
110+
// entries on screen. Anything past ERC1155_BATCH_DISPLAY_MAX is
111+
// still aggregated into the total below.
112+
if (context->array_index < ERC1155_BATCH_DISPLAY_MAX) {
113+
memcpy(context->batch_values[context->array_index], msg->parameter, INT256_LENGTH);
114+
}
101115
convertUint256BE(context->tokenId, sizeof(context->tokenId), &new_value);
102116
add256(&context->value, &new_value, &context->value);
103117
if (--context->values_array_len == 0) {

src/plugins/erc1155/erc1155_ui.c

Lines changed: 52 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,25 @@ static void set_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t *contex
8686
}
8787
}
8888

89+
// Screen indices for the SAFE_BATCH_TRANSFER review. Fixed-index screens
90+
// are mapped 1:1 to the enum; per-pair detail screens (PAIR_BASE..) and the
91+
// truncation warning use dynamic indices computed from batch_displayed.
92+
enum {
93+
BATCH_SCREEN_TO = 0,
94+
BATCH_SCREEN_COLLECTION,
95+
BATCH_SCREEN_NFT_ADDRESS,
96+
BATCH_SCREEN_TOTAL_QUANTITY,
97+
BATCH_SCREEN_PAIR_BASE,
98+
};
99+
89100
static void set_batch_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t *context) {
90101
char quantity_str[48];
102+
uint8_t idx = msg->screenIndex;
103+
uint8_t pair_screens = (uint8_t) (2 * context->batch_displayed);
104+
uint8_t warn_idx = (uint8_t) (BATCH_SCREEN_PAIR_BASE + pair_screens);
91105

92-
switch (msg->screenIndex) {
93-
case 0:
106+
switch (idx) {
107+
case BATCH_SCREEN_TO:
94108
strlcpy(msg->title, "To", msg->titleLength);
95109
if (!getEthDisplayableAddress(context->address,
96110
msg->msg,
@@ -99,11 +113,11 @@ static void set_batch_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t *
99113
msg->result = ETH_PLUGIN_RESULT_ERROR;
100114
}
101115
break;
102-
case 1:
116+
case BATCH_SCREEN_COLLECTION:
103117
strlcpy(msg->title, "Collection Name", msg->titleLength);
104118
strlcpy(msg->msg, msg->item1->nft.collectionName, msg->msgLength);
105119
break;
106-
case 2:
120+
case BATCH_SCREEN_NFT_ADDRESS:
107121
strlcpy(msg->title, "NFT Address", msg->titleLength);
108122
if (!getEthDisplayableAddress(msg->item1->nft.contractAddress,
109123
msg->msg,
@@ -112,11 +126,11 @@ static void set_batch_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t *
112126
msg->result = ETH_PLUGIN_RESULT_ERROR;
113127
}
114128
break;
115-
case 3:
129+
case BATCH_SCREEN_TOTAL_QUANTITY:
116130
strlcpy(msg->title, "Total Quantity", msg->titleLength);
117131
if (!tostring256(&context->value, 10, &quantity_str[0], sizeof(quantity_str))) {
118132
msg->result = ETH_PLUGIN_RESULT_ERROR;
119-
break;
133+
return;
120134
}
121135
snprintf(msg->msg,
122136
msg->msgLength,
@@ -125,8 +139,38 @@ static void set_batch_transfer_ui(ethQueryContractUI_t *msg, erc1155_context_t *
125139
context->array_index);
126140
break;
127141
default:
128-
PRINTF("Unsupported screen index %d\n", msg->screenIndex);
129-
msg->result = ETH_PLUGIN_RESULT_ERROR;
142+
// Per-pair detail screens, then optional truncation warning.
143+
if (idx >= BATCH_SCREEN_PAIR_BASE && idx < BATCH_SCREEN_PAIR_BASE + pair_screens) {
144+
uint8_t pair_offset = (uint8_t) (idx - BATCH_SCREEN_PAIR_BASE);
145+
uint8_t pair_idx = (uint8_t) (pair_offset / 2);
146+
bool show_value = (pair_offset % 2) == 1;
147+
if (show_value) {
148+
uint256_t v;
149+
convertUint256BE(context->batch_values[pair_idx], INT256_LENGTH, &v);
150+
snprintf(msg->title, msg->titleLength, "Quantity #%d", pair_idx + 1);
151+
if (!tostring256(&v, 10, msg->msg, msg->msgLength)) {
152+
msg->result = ETH_PLUGIN_RESULT_ERROR;
153+
}
154+
} else {
155+
snprintf(msg->title, msg->titleLength, "NFT ID #%d", pair_idx + 1);
156+
if (!uint256_to_decimal(context->batch_ids[pair_idx],
157+
INT256_LENGTH,
158+
msg->msg,
159+
msg->msgLength)) {
160+
msg->result = ETH_PLUGIN_RESULT_ERROR;
161+
}
162+
}
163+
} else if (context->batch_truncated && idx == warn_idx) {
164+
strlcpy(msg->title, "WARNING", msg->titleLength);
165+
snprintf(msg->msg,
166+
msg->msgLength,
167+
"Only first %d of %d IDs shown",
168+
ERC1155_BATCH_DISPLAY_MAX,
169+
context->array_index);
170+
} else {
171+
PRINTF("Unsupported screen index %d\n", idx);
172+
msg->result = ETH_PLUGIN_RESULT_ERROR;
173+
}
130174
break;
131175
}
132176
}
-3 Bytes
-10 Bytes
-2 Bytes
702 Bytes
-399 Bytes
1.39 KB

0 commit comments

Comments
 (0)