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

Dangerous strict equalities operation in DeliveryPlace::closeBidTaker, this leads to logical error in calculations

emma77

Summary

DeliveryPlace::closeBidTaker implementation has a line of code that executes a Dangerous strict equalities :

if (offerInfo.usedPoints > offerInfo.settledPoints) {
@> if (offerInfo.offerStatus == OfferStatus.Virgin) { // @audit Dangerous strict equalities
collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);
}

Vulnerability Details

function closeBidTaker(address _stock) external {
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
ITokenManager tokenManager = tadleFactory.getTokenManager();
StockInfo memory stockInfo = perMarkets.getStockInfo(_stock);
​
if (stockInfo.preOffer == address(0x0)) {
revert InvalidStock();
}
​
if (stockInfo.stockType == StockType.Ask) {
revert InvalidStockType();
}
​
if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
​
(
OfferInfo memory preOfferInfo,
MakerInfo memory makerInfo,
,
​
) = getOfferInfo(stockInfo.preOffer);
​
OfferInfo memory offerInfo;
uint256 userRemainingPoints;
@> if (makerInfo.offerSettleType == OfferSettleType.Protected) { //@audit Dangerous strict equalities
offerInfo = preOfferInfo;
userRemainingPoints = stockInfo.points;
} else {
offerInfo = perMarkets.getOfferInfo(makerInfo.originOffer);
if (stockInfo.offer == address(0x0)) {
userRemainingPoints = stockInfo.points;
} else {
OfferInfo memory listOfferInfo = perMarkets.getOfferInfo(
stockInfo.offer
);
userRemainingPoints =
listOfferInfo.points -
listOfferInfo.usedPoints;
}
}
​
if (userRemainingPoints == 0) {
revert InsufficientRemainingPoints();
}
​
if (offerInfo.offerStatus != OfferStatus.Settled) {
revert InvalidOfferStatus();
}
​
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus();
}
​
uint256 collateralFee;
if (offerInfo.usedPoints > offerInfo.settledPoints) {
@> if (offerInfo.offerStatus == OfferStatus.Virgin) { // @audit Dangerous strict equalities
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
);
}
}
​
uint256 userCollateralFee = collateralFee.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
​
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash,
_msgSender(),
makerInfo.tokenAddress,
userCollateralFee
);
uint256 pointTokenAmount = offerInfo.settledPointTokenAmount.mulDiv(
userRemainingPoints,
offerInfo.usedPoints,
Math.Rounding.Floor
);
tokenManager.addTokenBalance(
TokenBalanceType.PointToken,
_msgSender(),
makerInfo.tokenAddress,
pointTokenAmount
);
​
perMarkets.updateStockStatus(_stock, StockStatus.Finished);
​
emit CloseBidTaker(
makerInfo.marketPlace,
offerInfo.maker,
_stock,
_msgSender(),
userCollateralFee,
pointTokenAmount
);
}

Impact

This leads to error in calculation and an attacker can use this to exploit the protocol to drain funds/assets

Tools Used

manual review

Recommendations

Be Flexible with Conditions,

Avoid overly strict conditions that can’t be reasonably met within the blockchain’s constraints.

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.