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

2 stars 0 forks source link

SWC-118 Incorrect Constructor Name. WRONG NAME. Function Name is the same as Contract Name. #49

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/utils/Multicall.sol#L22

Vulnerability details

Impact

WRONG NAME. The Function Name is the same as Contract Name. One is proper case and the other is lower case. The contract name is Multicall and the function name within it is lower case multicall. So this opens up security vulnerabilities like the following. One of the countless impacts is that the owner can be changed to a malicious attacker. A second possibility is that calls can be made to withdraw funds if available in the account.

Proof of Concept

Provide direct links to all referenced code in GitHub:

https://github.com/code-423n4/2023-07-axelar/blob/2f9b234bb8222d5fbe934beafede56bfb4522641/contracts/its/utils/Multicall.sol#L22-L33

File

/utils/Multicall.sol#L22

Victim Address: 0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC Victim Start Balance: Balance: 10000 ETH Victim End Balance After Attack: Balance: 8888.99992770374008314 ETH

Attacker Address: 0x90F79bf6EB2c4f870365E785982E1f101E93b906 Attacker Start Balance: 10000 ETH Attacker End Balance After Attack: Balance: 11111 ETH

Log follows after POC.

Steps to reproduce

NB: Using Remix Dev Foundry Provider (run anvil in cmd and foundry needs to be installed)

  1. switch to first account (as alice) and deploy the victim contract.
  2. switch to second account (as eve) and deploy the attack contract passing deploy to address option as victim contract address.
  3. In the deploy section under Value enter 1111 ether. Then switch to attacker account for eve and in the multicall2 button enter the address of victim click multicall2() button.
  4. call to data is successful.
  5. And the interfaces IMulticall.(fallback) is called and 1111 ether is moved from the victim account to the attacker account.

PoC

// SPDX-License-Identifier: MIT

pragma solidity ^0.8.0;

import { IMulticall } from '../interfaces/IMulticall.sol';
import {Multicall} from '../utils/Multicall.sol';

contract MulticallDebo {

    Multicall public multicall; 
    bytes[] public debo;
    // bytes[] public datae = [bytes(abi.encodeWithSignature('0xh3xh3xh3xh3xh3xh3xh3xh3xh3xh3x'))];

    function multicall2(address _multicall) public payable {
        multicall = Multicall(_multicall);
        debo = [bytes(abi.encodeWithSignature('0xh3xh3xh3xh3xh3xh3xh3xh3xh3xh3x'))];
        multicall.multicall(debo);
    }

}

Log

[block:2 txIndex:0]from: 0x3C4...293BCto: IMulticall.(fallback) 0x90F...3b906value: 1111000000000000000000 weidata: 0x358...293bclogs: 0hash: 0x915...65614
status  true Transaction mined and execution succeed
transaction hash    0xb1efeb49d9901748d23887c4265ad4ca2b15a4858069765654d91194ffeb54ec
from    0x3C44CdDdB6a900fa2b585dd299e03d12FA4293BC
to  IMulticall.(fallback) 0x90F79bf6EB2c4f870365E785982E1f101E93b906
gas 21420 gas
transaction cost    21420 gas 
input   0x358...293bc
decoded input   -
decoded output   - 
logs    []
val 1111000000000000000000 wei

Tools Used

Mythx && VS Code && Remix && Foundry

Recommended Mitigation Steps

Solidity version 0.4.22 introduces a new constructor keyword that make a constructor definitions clearer. It is therefore recommended to upgrade the contract to a recent version of the Solidity compiler and change to the new constructor declaration.

Assessed type

call/delegatecall

0xSorryNotSorry commented 1 year ago

The submission does not provide any demonstration of the issue, reasoning and code blocks.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

berndartmueller marked the issue as unsatisfactory: Insufficient proof