Assessment reports>Alkimiya>Critical findings>The ,redeemShort, function is available before the pool is closed
Category: Coding Mistakes

The redeemShort function is available before the pool is closed

Critical Severity
Critical Impact
High Likelihood

Description

The redeemShort function allows the caller to redeem short token ID shares for the payout token. The short token ID shares of the caller will be burned, and the calculated number of payoutToken will be sent to the caller in exchange. The amount of payoutToken tokens depends on the balanceChangePerShare, which is set only when the pool is closed; until then, the value will be zero. But, since this function does not check that the pool has already been closed, this function can be successfully executed.

function redeemShort(PoolParams calldata shortParams) public {
    bytes32 poolHash = hashPool(shortParams);
    uint256 shortTokenId = toShortTokenId(poolHash);
    ...
    PoolState storage sState = sPoolState[poolHash];
    ...
    uint256 payout =
        uint256(shortParams.cap - sState.balanceChangePerShare) * uint256(sState.collateralMinted) / relativeAmount;
    ...
    }

Impact

The payout for short positions will be maximum for the case where balanceChangePerShare is zero — that is, the caller wrongly receives the maximum payout when this function is called before the pool is closed.

Recommendations

We recommend adding a check to ensure that the redeemShort function is explicitly called only after the pool is closed, when the balanceChangePerShare value is set. We also recommend adding such a check to the redeemLong function. Although the function cannot be executed with zero balanceChangePerShare because of reverting during calculation, having an explicit check will help to avoid unexpected behavior or mistakes in the future.

Remediation

This issue has been acknowledged by Alkimiya, and a fix was implemented in commit 0781fb97.

Zellic © 2024Back to top ↑