Skip to content

Commit db1a3d9

Browse files
Cerberus Merlincedelavergne-ledger
authored andcommitted
fix: ERC1155 safeBatchTransferFrom parser trusts unvalidated dynamic offsets and can spoof the review UI
1 parent f104fb0 commit db1a3d9

1 file changed

Lines changed: 30 additions & 16 deletions

File tree

src/plugins/erc1155/erc1155_provide_parameters.c

Lines changed: 30 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,19 @@ static void handle_batch_transfer(ethPluginProvideParameter_t *msg, erc1155_cont
5353
context->next_param = TOKEN_IDS_LENGTH;
5454
break;
5555
case TOKEN_IDS_LENGTH:
56-
if ((msg->parameterOffset + PARAMETER_LENGTH) > context->ids_offset) {
57-
context->ids_array_len =
58-
U2BE(msg->parameter, PARAMETER_LENGTH - sizeof(context->ids_array_len));
59-
context->next_param = TOKEN_ID;
60-
// set to zero for next step
61-
context->array_index = 0;
56+
if (msg->parameterOffset < context->ids_offset) {
57+
// not there yet
58+
break;
6259
}
60+
if (msg->parameterOffset != context->ids_offset) {
61+
msg->result = ETH_PLUGIN_RESULT_ERROR;
62+
break;
63+
}
64+
context->ids_array_len =
65+
U2BE(msg->parameter, PARAMETER_LENGTH - sizeof(context->ids_array_len));
66+
context->next_param = TOKEN_ID;
67+
// set to zero for next step
68+
context->array_index = 0;
6369
break;
6470
case TOKEN_ID:
6571
// don't copy anything since we won't display it
@@ -69,17 +75,25 @@ static void handle_batch_transfer(ethPluginProvideParameter_t *msg, erc1155_cont
6975
context->array_index++;
7076
break;
7177
case VALUE_LENGTH:
72-
if ((msg->parameterOffset + PARAMETER_LENGTH) > context->values_offset) {
73-
context->values_array_len =
74-
U2BE(msg->parameter, PARAMETER_LENGTH - sizeof(context->values_array_len));
75-
if (context->values_array_len != context->array_index) {
76-
PRINTF("Token ids and values array sizes mismatch!");
77-
}
78-
context->next_param = VALUE;
79-
// set to zero for next step
80-
context->array_index = 0;
81-
explicit_bzero(&context->value, sizeof(context->value));
78+
if (msg->parameterOffset < context->values_offset) {
79+
// not there yet
80+
break;
81+
}
82+
if (msg->parameterOffset != context->values_offset) {
83+
msg->result = ETH_PLUGIN_RESULT_ERROR;
84+
break;
8285
}
86+
context->values_array_len =
87+
U2BE(msg->parameter, PARAMETER_LENGTH - sizeof(context->values_array_len));
88+
if (context->values_array_len != context->array_index) {
89+
PRINTF("Token ids and values array sizes mismatch!");
90+
msg->result = ETH_PLUGIN_RESULT_ERROR;
91+
break;
92+
}
93+
context->next_param = VALUE;
94+
// set to zero for next step
95+
context->array_index = 0;
96+
explicit_bzero(&context->value, sizeof(context->value));
8397
break;
8498
case VALUE:
8599
// put it temporarily in token id since we don't use it in batch transfer

0 commit comments

Comments
 (0)