Summary

https://github.com/Cyfrin/2024-05-TSender/blob/c6da9ef0c28741c007a02dfa07b7e899c1c22e47/src/protocol/TSender.huff#L204-L283
The macro checks if the total amount specified in the calldata matches the sum of the amounts array. However, the logic to sum up the amounts and compare it to the totalAmount specified is not explicitly shown or may be incorrectly implemented.

Vulnerability Details

`#define macro ARE_LISTS_VALID() = takes (0) returns (0) {
// The recipients starting location will always be 0x44
[RECIPIENTS_LENGTH_OFFSET] calldataload // [recipients.length]
dup1 // [recipients.length, recipients.length]
dup1 // [recipients.length, recipients.length, recipients.length]
// check if the number of recipients is 0
0x00 eq return_false jumpi // [recipients.length, recipients.length]

// However, the amounts starting location may be different depending on the number of recipients
// Check if the number of recipients matches the number of amounts
[AMOUNTS_OFFSET_OFFSET] calldataload     // [location_of_amounts_offset (without function selector), recipients.length, recipients.length]
0x04 add                                 // [location_of_amounts_offset (with function selector), recipients.length, recipients.length]
calldataload                             // [amounts.length, recipients.length, recipients.length]
eq iszero return_false jumpi             // [recipients.length]


0x00                                     // [outer_loop_count, recipients.length]
dup2 dup2                                // [outer_loop_count, recipients.length, outer_loop_count, recipients.length]


outer_loop_start:                        // [inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
    // Break loop conditional
    eq return_true jumpi               // [outer_loop_count, recipients.length]


    // Check for zero address
    dup1                               // [loop_count, loop_count, recipients.length]
    0x20 mul                           // [outer_loop_count * 32, recipients.length]
    [ADDRESS_ONE_OFFSET] add           // [current_address_offset, outer_loop_count, recipients.length]
    calldataload                       // [outer_loop_address, outer_loop_count, recipients.length]
    dup1                               // [outer_loop_address, outer_loop_address, outer_loop_count, recipients.length]
    iszero return_false jumpi          // [outer_loop_address, outer_loop_count, recipients.length]


    // Check for zero amount
    // This is not gas efficient at all, whatever
    dup2                                  // [outer_loop_count, outer_loop_address, outer_loop_count, recipients.length]
    0x20 mul                              // [outer_loop_count * 32, outer_loop_address, outer_loop_count, recipients.length]
    [AMOUNTS_OFFSET_OFFSET] calldataload  // [location_of_amounts_offset (without function selector), outer_loop_count * 32, outer_loop_address, outer_loop_count, recipients.length]
    // we do 0x24 here since we need to step over amounts.length + the function selector 
    0x24 add                              // [location_of_amounts_offset (with function selector), outer_loop_count * 32, outer_loop_address, outer_loop_count, recipients.length]
    add calldataload                      // [amounts[i], outer_loop_count * 32, outer_loop_address, outer_loop_count, recipients.length]
    iszero return_false jumpi             // [outer_loop_address, outer_loop_count, recipients.length]


    // inner loop starts at index i + 1
    dup2 0x01 add                      // [inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]


    inner_loop_start:
        // Break loop conditional
        dup1 dup5                      // [recipients.length, inner_loop_count, inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
        eq inner_loop_end jumpi        // [inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
        // Compare addresses
        dup1                           // [inner_loop_count, inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
        0x20 mul                       // [inner_loop_count * 32, inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]   
        [ADDRESS_ONE_OFFSET] add       // [inner_loop_address_offset, inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]   
        calldataload                   // [inner_loop_address, inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
        dup3                           // [outer_loop_address, inner_loop_address, inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
        // We could obviously combine these next two jumps into one for gas optimization
        eq return_false jumpi          // [inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]


        // Incremement inner loop count
        0x01 add                       // [inner_loop_count + 1, outer_loop_address, outer_loop_count, recipients.length]
        inner_loop_start jump


        // This is a gross waste of gas having all these jumps... but whatever
        inner_loop_end:                    // [inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
            // pop off inner loop vars
            pop pop 
            // Increment outer loop count
            0x01 add                       // [outer_loop_count + 1, recipients.length]
            dup2 dup2                      // [outer_loop_count, recipients.length, outer_loop_count, recipients.length]
            outer_loop_start jump
        // inner_loop target end stack:// [inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
// outer_loop target end stack: // [inner_loop_count, outer_loop_address, outer_loop_count, recipients.length]
return_true:
    // 1 is true
    0x01 0x00 mstore 
    0x20 0x00 return

return_false:
    // 0 is false
    0x00 0x00 mstore 
    0x20 0x00 return

}`

Impact

This can lead to scenarios where the total amount transferred does not match the sum specified, potentially leading to incorrect transaction amounts being processed

Tools Used

Manual Review

Recommendations

Ensure that each recipient in the recipients array has a corresponding amount in the amounts array by checking the lengths and contents of both arrays more thoroughly.