The redeemShort
function is available before the pool is closed
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↗.