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

Logical Flaw in Collateral Fee Calculation

thopatevijay

Vulnerability Details

In the closeBidTaker function, there's a potential logical flaw in the control flow. The function first checks if offerInfo.usedPoints > offerInfo.settledPoints, and only then checks the offerStatus. This order of operations could lead to unexpected behavior, as the offerStatus check is nested within the points comparison.

Impact

This logical issue could result in incorrect calculation of the collateralFee. Depending on the offer's status and the points comparison, the wrong branch of code might be executed, leading to an incorrect fee calculation. This could potentially be exploited by malicious actors to manipulate the collateral fees in their favor, or it could simply lead to unintended behavior that disrupts the normal operation of the contract.

Proof Of Concept

Link to code

function closeBidTaker(address _stock) external {
// ... [earlier code omitted for brevity]
​
if (offerInfo.usedPoints > offerInfo.settledPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);
} else {
// This branch is never reached if usedPoints <= settledPoints,
// even if the offer is not in Virgin status
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
​
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
}
// If usedPoints <= settledPoints, collateralFee remains 0
​
// ... [rest of the function]
}

In this scenario, if offerInfo.usedPoints <= offerInfo.settledPoints, the collateralFee will always be 0, regardless of the offer's status. This could lead to incorrect fee calculations and potential loss of funds.

Tools Used

Recommendations

  • Reorder the checks to prioritize the offer status before comparing points:

if (offerInfo.offerStatus == OfferStatus.Virgin) {
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);
} else if (offerInfo.usedPoints > offerInfo.settledPoints) {
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
​
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
} else {
// Handle the case where usedPoints <= settledPoints
// This might involve setting collateralFee to 0 or calculating it differently
}

  • Add explicit handling for all possible combinations of offer status and points comparison to ensure all cases are properly accounted for.

  • Consider adding additional checks and require statements to validate the state of the offer before proceeding with calculations.

Updates

Lead Judging Commences

0xnevi Lead Judge
about 1 year ago
0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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