Division before multiplication could lead to users losing 50% in WithdrawalQueue

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/WithdrawalQueue.sol#L137-L144

Vulnerability details

Impact

In the _getAvailable() function, the calculation performs division before multiplication, which could result in precision loss. The consequence is that users may not be able to withdraw the amount they should receive, leaving some funds locked in the WithdrawalQueue.

// @audit division before multiplication
function _getAvailable(uint256 _tokenId) private view returns (uint256) {
    return getShares[_tokenId] * _getWithdrawablePerShare() - getWithdrawn[_tokenId]; 
}

/// @notice Get the amount that can be withdrawn per share.
function _getWithdrawablePerShare() private view returns (uint256) {
    return (_totalWithdrawn + _asset.balanceOf(address(this))) / getTotalShares;
}

Proof of Concept

Consider the following scenario:

getShares[_tokenId] = 1e8
getWithdrawn[_tokenId] = 0
_totalWithdrawn = 0
_asset.balanceOf(address(this)) = 1e9 (1000 USDC)
getTotalShares = 5e8 + 1

The current calculation will yield

_getWithdrawablePerShare() = 1e9 / (5e8 + 1) = 1
_getAvailable() = 1e8 * 1 - 0 = 1e8 = 100000000

However, the users should actually receive

getShares[_tokenId] * _asset.balanceOf(address(this)) / getTotalShares
= 1e8 * 1e9 / (5e8 + 1) = 199999999

As shown, the users lose almost 50% of what they should receive.

Tools Used

Manual Review

Recommended Mitigation Steps

Change the order of calculation to multiply before division.

Assessed type

Math

[c4-judge]

0xA5DF marked the issue as primary issue

[c4-judge]

0xA5DF marked the issue as selected for report

[c4-judge]

0xA5DF marked the issue as satisfactory

[0xend]

https://github.com/pixeldaogg/florida-contracts/pull/362