Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Users can not return their collateral if their `offerInfo.usedPoints` is zero

pontifex

Summary

An unreachable condition in the DeliveryPlace.closeBidTaker function can cause collateral locking.

Vulnerability Details

The DeliveryPlace.closeBidTaker can be executed only then offerInfo.offerStatus == OfferStatus.Settled. Since collateralFee depends on the offerInfo.usedPoints the case when no one used the offer points the offerInfo.usedPoints stays 0. This should be handled with the if (offerInfo.offerStatus == OfferStatus.Virgin) { branch but this condition is impossible. This way flow goes in another branch and the collateralFee will be 0.

function closeBidTaker(address _stock) external {
<...>
>> if (offerInfo.offerStatus != OfferStatus.Settled) {
revert InvalidOfferStatus();
}
​
uint256 collateralFee;
if (offerInfo.usedPoints > offerInfo.settledPoints) {
>> if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);
} else {
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
​
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
}
​

Impact

Assets blocking, unexpected behavior

Tools used

Manual Review

Recommendations

Consider checking if offerInfo.usedPoints is zero instead of offerInfo.offerStatus == OfferStatus.Virgin

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

[invalid] finding-PreMarkets-closeBidTaker-Virgin-Settled-unreachable

Borderline informational/low severity, taker bid offers can only be closed after settlement by original makers, so the check for `Settled` offer status is correct but the initial `if` block is dead code and will never be reached i.e., even if original maker offer was not settled, this issue cannot be exploited. Additionally, makers are incentivized to settle original offers to earn maker bonuses from subsequent trades from the original maker offers by takers. Some issues such as 612, 1774 and 1775 have no impact described but I am duplicating anyways since I am invalidating this issue. Assigning as informational severity since I believe this can be seen as simply a waste of gas and confusing code logic.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.