c4-bot-7 commented 9 months ago

Lines of code

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

Vulnerability details

Impact

During the upkeep process, when invoking withdrawPOL, the function collateralAndLiquidity.withdrawLiquidityAndClaim fails to specify the values for minReclaimedA and minReclaimedB. This oversight implies that zero amounts of tokenA and tokenB could be received when executing the liquidity removal. Malicious users can exploit this by front-running the withdrawPOL process, influencing the amount of reclaimable tokens for the DAO, and potentially profiting from the situation.

Proof of Concept

withdrawPOL sets the minReclaimedA and minReclaimedB to ZERO in function withdrawLiquidityAndClaim.

    function withdrawPOL( IERC20 tokenA, IERC20 tokenB, uint256 percentToLiquidate ) external
        {
        require(msg.sender == address(liquidizer), "DAO.withdrawProtocolOwnedLiquidity is only callable from the Liquidizer contract" );

        bytes32 poolID = PoolUtils._poolID(tokenA, tokenB);
        uint256 liquidityHeld = collateralAndLiquidity.userShareForPool( address(this), poolID );
        if ( liquidityHeld == 0 )
            return;

        uint256 liquidityToWithdraw = (liquidityHeld * percentToLiquidate) / 100;

        // Withdraw the specified Protocol Owned Liquidity
//@audit no slippage protect
        (uint256 reclaimedA, uint256 reclaimedB) = collateralAndLiquidity.withdrawLiquidityAndClaim(tokenA, tokenB, liquidityToWithdraw, 0, 0, block.timestamp );

        // Send the withdrawn tokens to the Liquidizer so that the tokens can be swapped to USDS and burned as needed.
        tokenA.safeTransfer( address(liquidizer), reclaimedA );
        tokenB.safeTransfer( address(liquidizer), reclaimedB );

        emit POLWithdrawn(tokenA, tokenB, reclaimedA, reclaimedB);
        }

In the given scenario:

Initially, for the DAO, totalLiquidity is 100, liquidityToRemove is 1, and reserve0 is 100.
A malicious user introduces a liquidation, causing totalLiquidity to increase to 1000, with reserve0 now at 900.
When withdrawPOL invokes withdrawLiquidityAndClaim, it obtains 900 * 1/1000 = 0 tokenA.
Exploiting this situation, the malicious user removes the liquidation to gain profits.

Tools Used

Manual Review

Recommended Mitigation Steps

Set the minReclaimedA and minReclaimedB in the function withdrawPOL.

Assessed type

Invalid Validation

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #344

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory