Tadle

Summary

There are two different checks in closeBidTaker function if one of them is true then other cannot be executed. So either one is wrong.

Vulnerability Details

Following is a snippet of closeBidTaker function

As can be seen from the following check that it requires the offer to be settled.

Now there is also the following check

the above if condition would be executed when offer status is virgin.

The main logic behind this if condition is that when the maker doesn't settles his order correctly then his collateral would be stuck in the protocol so now the bidder can close his offer in order to get back his bid amount. As maker didn't correctly settles his offer so the leftover collateral should be distributed among all the bidders of this offer but as initially there is check which enforces that the offer status should be settled therefore this if condition wouldn't be executed thus there will be some left over collateral stuck in the contract forever.

Impact

The main logic behind the second if condition can't be executed thus there will be collateral stuck in the contract.

Tools Used

Manual Review.

Recommendations

Mitigation for this is very simple because if the make doesn't settles his offer then its whole collateral is stuck in the contract so that should be distributed among the bidders. It should be distributed by the amount of tokens each bidder holds.

So do the following changes in the closebidtaker function by just removing the else condition and also removing the if (offerInfo.offerStatus == OfferStatus.Virgin) condition.