Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Ask offers on Turbo mode can be used to drain contract funds via `DeliveryPlace::settleAskMaker`

honour

Summary

Turbo mode allows subsequent Ask offers to be created without a need for collateral to be deposited, relying on the collateral provided by the original offer maker.
DeliveryPlace::settleAskMaker however does not verify the mode of the offer to be settled and blindly refunds the offer maker the amount of the offer.

Vulnerability Details

If offerSettleType is Turbo collateral is not required as seen here, so the amount set is up to the caller and can be arbitarily large(since they don' have to deposit collateral).
DeliveryPlace::settleAskMaker does not verify the settle type of the offer
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L275-L307

uint256 makerRefundAmount;
if (_settledPoints == offerInfo.usedPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
makerRefundAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,//ask so makerRefundAmount = amount * collateralRate
offerInfo.collateralRate,
offerInfo.amount,
true,
Math.Rounding.Floor
);
} else {//cancelled. canceling returns amount for points not used, so here we return for used points
uint256 usedAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
​
makerRefundAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
usedAmount,
true,
Math.Rounding.Floor
);
}
​
tokenManager.addTokenBalance(
TokenBalanceType.SalesRevenue,
_msgSender(),
makerInfo.tokenAddress,
makerRefundAmount
);
}

From the code ,assuming _settledPoints == offerInfo.usedPoints == 0 (in reality, amount/point ratio will be too large for the offer to be taken anyways) and offerInfo.offerStatus == OfferStatus.Virgin then makerRefundAmount = offerInfo.amount * collateralRate and the attacker can completely drain contract funds with the right amount * collaterate

Impact

HIGH/CRITICAL - An attacker can set a large enough amount (as offer.amount) to drain contract funds

Tools Used

Manual Review

Recommendations

Add check to disallow settling of turbo offers if not the originOffer.

function settleAskMaker(address _offer, uint256 _settledPoints) external {
(
OfferInfo memory offerInfo,
MakerInfo memory makerInfo,
MarketPlaceInfo memory marketPlaceInfo,
MarketPlaceStatus status
) = getOfferInfo(_offer);
​
if (_settledPoints > offerInfo.usedPoints) {
revert InvalidPoints();
}
​
if (marketPlaceInfo.fixedratio) {
revert FixedRatioUnsupported();
}
​
if (offerInfo.offerType == OfferType.Bid) {
revert InvalidOfferType(OfferType.Ask, OfferType.Bid);
}
​
+ if (makerInfo.offerSettleType == OfferSettleType.Turbo && makerInfo.originOffer != _offer) {
+ revert("invalid settle status");
+ }
​
if (
offerInfo.offerStatus != OfferStatus.Virgin &&
offerInfo.offerStatus != OfferStatus.Canceled
) {
revert InvalidOfferStatus();
}
//...ommited for brevity
​
}
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-Premarkets-listOffer-turbo-settleAskMaker-exploit-settlement

Valid high severity, this allows resellers listing offers via `listOffer/relistOffer` to game the system. Based on the inherent design of Turbo mode not requiring takers making ask offers for the original maker offer to deposit collateral, the wrong refund of collateral to takers even when they did not deposit collateral due to turbo mode during settleAskMaker allows possible draining of pools.

Support

FAQs

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