code423n4 commented 1 year ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Multicall/Multicall3.sol#L41-L61

Vulnerability details

Impact

stranded ETH in Multicall3 contract

Proof of Concept

Any native tokens sent to the contract will be stuck in the contract with no way to rescue them https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/Multicall/Multicall3.sol#L41-L61

    function multicall(
        Call[] calldata calls
    ) public payable returns (Result[] memory returnData) {
        uint256 length = calls.length;
        returnData = new Result[](length);
        Call memory calli;
        for (uint256 i = 0; i < length; ) {
            Result memory result = returnData[i];
            calli = calls[i];

            (result.success, result.returnData) = calli.target.call(
                calli.callData
            );
            if (!result.success) {
                _getRevertMsg(result.returnData);
            }
            unchecked {
                ++i;
            }
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

remove payable keyword from the function

Assessed type

Payable

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-sponsor commented 1 year ago

0xRektora marked the issue as disagree with severity

0xRektora commented 1 year ago

Low. We use this internally for contract deployment.

c4-sponsor commented 1 year ago

0xRektora marked the issue as sponsor confirmed

c4-judge commented 1 year ago

dmvt changed the severity to QA (Quality Assurance)