There are two types of withdrawals in the protocol full and partial withdrawals. Full are when amount of shares to withdraw are greater than queued shares remaining and when it is otherwise around then it is partial withdrawal.
This is done in the first check the function _finalizeWithdrawals function does as we can see here:
The shares are deducted and sharesTowithdraw
have new deducted value now.
The issue here is that then function does not proceed and do full finalized withdrawal accounting , instead it checks if sharesRemaining
are bigger than new updated sharesToWithdraw
and based on outcome of that second check it will decide if it will do full or partial withdrawal as we can see here:
In order to see why this is issue lets check the following PoC:
For the sake of simplicity I will use following values:
sharesToWithdraw = 10
sharesRemaining = 7
First it check if sharesRemaining < sharesToWithdraw
which in our case it is true because 7 < 10
.
This should mean that withdrawal should be accounted as full withdrawal.
Then it deduct that amount and sharesToWithdraw
now have value of 3 (10 - 7 = 3).
Instead of do the full finalized withdrawal as it should instead it do another check if sharesRemaining > sharesToWithdraw
which now would be true also because 7 > 3
and it will proceed executing with partial withdrawal instead of full withdrawal as it should have done.
So instead of someone receiving full amount as it should it will receive way less (only 3 shares in our example), but the full amount of 7 shares would be deducted from him as we could see in the example above. Which in the end will make users incur losses and not be able to withdraw correct amount due to incorrect accounting of the protocol.
The _finalizeWithdrawals
is used by performUpkeep
and by deposit
which is called from PriotityPool
, so in the end whole protocol can be impacted due to incorrect accounting and wrong data can be passed.
Incorrect implementation of _finalizeWithdrawals
could make users incur losses and not be able to withdraw correct amount due to incorrect accounting and wrong data can be passed to the protocol.
Manual Review
Do a full finalized withdrawal accounting if sharesRemaining
are less than sharesToWithdraw
immediatelly after deducting shares like this:
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.