Skip to content

Commit 60d6391

Browse files
author
Cerberus Merlin
committed
fix: ERC1155 safeBatchTransferFrom parser trusts unvalidated dynamic offsets and can spoof the review UI
1 parent 1fbc2ac commit 60d6391

1 file changed

Lines changed: 18 additions & 4 deletions

File tree

src/plugins/erc1155/erc1155_provide_parameters.c

Lines changed: 18 additions & 4 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) {
56+
if (msg->parameterOffset < context->ids_offset) {
57+
// not there yet
58+
break;
59+
}
60+
if (msg->parameterOffset != context->ids_offset) {
61+
msg->result = ETH_PLUGIN_RESULT_ERROR;
62+
break;
63+
}
5764
context->ids_array_len =
5865
U2BE(msg->parameter, PARAMETER_LENGTH - sizeof(context->ids_array_len));
5966
context->next_param = TOKEN_ID;
6067
// set to zero for next step
6168
context->array_index = 0;
62-
}
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) {
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;
85+
}
7386
context->values_array_len =
7487
U2BE(msg->parameter, PARAMETER_LENGTH - sizeof(context->values_array_len));
7588
if (context->values_array_len != context->array_index) {
7689
PRINTF("Token ids and values array sizes mismatch!");
90+
msg->result = ETH_PLUGIN_RESULT_ERROR;
91+
break;
7792
}
7893
context->next_param = VALUE;
7994
// set to zero for next step
8095
context->array_index = 0;
8196
explicit_bzero(&context->value, sizeof(context->value));
82-
}
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)