incorrect implementation of remainingAmount in abortAskOffer leads to loss of funds
Summary
The calculations of how much a user can take out during aborting, was incorrectly implemented, as the function assumed remaining amount = used amount leading to taking more money out than it should allow to be taken out
Vulnerability Details
Main cause exists in the remaningAmount calculations, where it thinks that remaning amount is the amount that is allowed to be taken out, but a closer look at the calculation, tells us that it actually returns the part of the collateral that cant be removed and have being used by the used points,This allows transfer amount to take a large amount of the collateral deposited
But the real effect is shown above there where we can see the correct implementation of used amount and not remainingAmount, but where the issue actually is wrong is when calculating the totalDepositAmount here it is not correct to use totalUsedAmount, but to use offerInfo.amount - totalUsedAmount which will produce, the actual remaning amount that can be taken out
The wrong calculations will produce a wrong makerRefund amount
For example to get the amount a user can take from his collateral, we consider this example
points - 1000
amount -10000
cr-12000
used points 600
according to this function remainingAmount should be = 10000 * 600/1000 = 6000( This is used amount , this is the amount you cant take out),
transferAmount will be = 7200
usedAmount is also calculated that way = 6000, totalDepositAmount will return 6000 also
The correct calculation should be , offer.amount - usedAmount = 10000 - 6000 = 4000, As a result Maker refund will return a completley incorrect amount
Impact
The collateral deposits of the askMAker protects the takers from loss of funds,and also protocol accounting erros if they can withdraw more than they should it could lead to loss of funds for the takers
Tools Used
Manual Review
Recommendations
remaningAmount should be calculated as usedAmount, and the difference between the offerAmount should be used to calculated the Refund amount
Lead Judging Commences
[invalid] finding-PreMarkets-abortAskOffer-remainingAmount-compute
Valid high, for cancelled offers, the unused collateral should be returned back to the maker. The `remainingAmount` is calculated wrongly with regards to usedPoints instead of unused points. Note: See comments under 826 and 907 for invalidation reasons
[invalid] finding-PreMarkets-abortAskOffer-makerRefundAmount-wrong-compute
Note, #148, #826, #1784 all stems from the fact that order statuses are not appropriately updated when a taker order is created against a maker offer. If the status is switched to `Ongoing/Filled` respectively, the virgin checks will fail appropriately and subsequently all this issues will be fixed. They could possibly be duplicates, so leaving open for appeal. This actually is closely related to issue #148. If a taker has created an Bid/Ask offer agains a original maker offer and the Status is adjusted to Ongoing/Filled accordingly, then this wrong refund won't occur, so could be duplicates, given only Virgin orders can be aborted (same applies for canceled orders, since only virgin orders are supposed to be able to be canceled). Since the status update is incorrect, then if a virgin order that is partially filled is aborted, the collateral is not returned correctly even though the order is not settled, causing a loss of funds to makers.
Appeal created
[invalid] finding-PreMarkets-abortAskOffer-remainingAmount-compute
Valid high, for cancelled offers, the unused collateral should be returned back to the maker. The `remainingAmount` is calculated wrongly with regards to usedPoints instead of unused points. Note: See comments under 826 and 907 for invalidation reasons
[invalid] finding-PreMarkets-abortAskOffer-makerRefundAmount-wrong-compute
Note, #148, #826, #1784 all stems from the fact that order statuses are not appropriately updated when a taker order is created against a maker offer. If the status is switched to `Ongoing/Filled` respectively, the virgin checks will fail appropriately and subsequently all this issues will be fixed. They could possibly be duplicates, so leaving open for appeal. This actually is closely related to issue #148. If a taker has created an Bid/Ask offer agains a original maker offer and the Status is adjusted to Ongoing/Filled accordingly, then this wrong refund won't occur, so could be duplicates, given only Virgin orders can be aborted (same applies for canceled orders, since only virgin orders are supposed to be able to be canceled). Since the status update is incorrect, then if a virgin order that is partially filled is aborted, the collateral is not returned correctly even though the order is not settled, causing a loss of funds to makers.
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.