Withdraw amount returned by getLiquidityAmountsAndPositions
may be incorrect
#452
Labels
2 (Med Risk)
Assets not at direct risk, but function/availability of the protocol could be impacted or leak value
bug
Something isn't working
disagree with severity
Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments)
M-01
primary issue
Highest quality submission among a set of duplicates
selected for report
This submission will be included/highlighted in the audit report
sponsor confirmed
Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
sufficient quality report
This report is of sufficient quality
Lines of code
https://github.com/code-423n4/2023-12-autonolas/blob/main/lockbox-solana/solidity/liquidity_lockbox.sol#L377
Vulnerability details
Impact
The
getLiquidityAmountsAndPositions()
function in theliquidity_lockbox
contract is used to calculate the liquidity amounts and positions to be withdrawn for a given total withdrawal amount. It iterates through each deposited position following a FIFO order as shown below:However, there is an error in the calculation of the last position amount when it entails a partial withdrawal. The
amountLeft
variable represents the leftover liquidity in the last position after the user's withdrawals, but it is assigned to the returned amounts as if it represented the amount the user should withdraw:This discrepancy could lead to users withdrawing an incorrect amount if they use
getLiquidityAmountsAndPositions()
, as intended, to obtain positions and amounts to use when callingwithdraw()
. This can result in significant unintended transfer of funds for the user, especially in cases where the last position inpositionAmounts
is of significant size. Given the fact that this issue can occur under normal usage of the contract, this issue is assessed as medium severity.Proof of Concept
Consider the following step-by-step scenario:
getLiquidityAmountsAndPositions()
to calculate the positions and amounts for the withdrawal and iterates through the returned arrays in a series of calls towithdraw()
.firstAvailablePositionAccountIndex
points to a very large position, which causesgetLiquidityAmountsAndPositions()
to return an amount much larger than Alice intended.withdraw
and as a result ends up withdrawing far more tokens than she intended, potentially depleting her liquidity in the contract.Tools Used
Manual review
Recommended Mitigation Steps
To fix this issue, the last position amount should be reduced by the remaining amount when the accumulated liquidity is not fully allocated. This can be achieved by changing the assignment operation to a subtraction operation:
This change ensures that the last position amount is correctly calculated, preventing users from withdrawing an incorrect amount.
Assessed type
Other
The text was updated successfully, but these errors were encountered: