code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

When forming POL the DAO will end up stucked with DAI and USDS tokens that cannot handle. #324

Open c4-bot-9 opened 9 months ago

c4-bot-9 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L321

Vulnerability details

Impact

The DAO contract cannot handle any token beside SALT tokens. So, if tokens like USDS or DAI were in its balance, they will be lost forever.

This can happen during the upkeep calls. Basically, during upkeep the contract takes some percentage from the arbitrage profits and use them to form POL for the DAO (usds/dai and salt/usds). The DAO swaps the ETH for both of the needed tokens and then adds the liquidity using the zapping flag to true.

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L316-L324

Zapping will compute the amount of either tokenA or tokenB to swap in order to add liquidity at the final ratio of reserves after the swap. But, it is important to note that the zap computations do no take into account that the same pool may get arbitraged atomically, changing the ratio of reserves a little.

As a consequence, some of the USDS and DAI tokens will be send back to the DAO contract:

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/Liquidity.sol#L110C3-L115C1

Proof of Concept

The following coded PoC should be pasted into root_tests/Upkeep.t.sol, actually its the same test that can be found at the end of the file with some added lines to showcase the issue. Specifically, it shows how the USDS and DAI balance of the DAO are zero before upkeep and how both are greater than zero after upkeep.

function testDoublePerformUpkeep() public {

        _setupLiquidity();
        _generateArbitrageProfits(false);

        // Dummy WBTC and WETH to send to Liquidizer
        vm.prank(DEPLOYER);
        weth.transfer( address(liquidizer), 50 ether );

        // Indicate that some USDS should be burned
        vm.prank( address(collateralAndLiquidity));
        liquidizer.incrementBurnableUSDS( 40 ether);

        // Mimic arbitrage profits deposited as WETH for the DAO
        vm.prank(DEPLOYER);
        weth.transfer(address(dao), 100 ether);

        vm.startPrank(address(dao));
        weth.approve(address(pools), 100 ether);
        pools.deposit(weth, 100 ether);
        vm.stopPrank();

        // === Perform upkeep ===
        address upkeepCaller = address(0x9999);

        uint256 daiDaoBalanceBefore = dai.balanceOf(address(dao));
        uint256 usdsDaoBalanceBefore = usds.balanceOf(address(dao));

        assertEq(daiDaoBalanceBefore, 0);
        assertEq(usdsDaoBalanceBefore, 0);

        vm.prank(upkeepCaller);
        upkeep.performUpkeep();
        // ==================

        _secondPerformUpkeep();

        uint256 daiDaoBalanceAfter = dai.balanceOf(address(dao));
        uint256 usdsDaoBalanceAfter = usds.balanceOf(address(dao));

        assertTrue(daiDaoBalanceAfter > 0);
        assertTrue(usdsDaoBalanceAfter > 0);
}

Tools Used

Manual Review

Recommended Mitigation Steps

The leftovers of USDS or DAI should be send to liquidizer so they can be handled.

Assessed type

Other

c4-judge commented 8 months ago

Picodes marked the issue as duplicate of #721

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory

c4-judge commented 8 months ago

Picodes marked the issue as selected for report

c4-sponsor commented 8 months ago

othernet-global (sponsor) confirmed

othernet-global commented 8 months ago

The DAO contract uses the available token balances to form POL, ensuring no extra tokens left in the contract.

https://github.com/othernet-global/salty-io/commit/5364426aaf97e646fa3990f148e364167adcd0a5

othernet-global commented 8 months ago

POL has been removed from the protocol

eaf40ef0fa27314c6e674db6830990df68e5d70e https://github.com/othernet-global/salty-io/commit/8e3231d3f444e9851881d642d6dd03021fade5ed