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

settleAskMaker() will not properly settle offer if maker settles a chunk of the usedPoints

stanchev

Summary

settleAskMaker() will not properly settle offer if _settledPoints < offerInfo.usedPoints, because checks are not enforced properly. This will result in permanently locked collateral of the maker.

Vulnerability Details

If maker wants to settle part of his used points his collateral will be locked forever.

The sequence of events allow the offer to be settled at the end of the function but the collateral will stay locked.

if (_settledPoints == offerInfo.usedPoints) {
if (offerInfo.offerStatus == OfferStatus.Virgin) {
makerRefundAmount = 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);
​
makerRefundAmount = OfferLibraries.getDepositAmount(offerInfo.offerType, offerInfo.collateralRate, usedAmount, true, Math.Rounding.Floor);
}
​
tokenManager.addTokenBalance(TokenBalanceType.SalesRevenue, _msgSender(), makerInfo.tokenAddress, makerRefundAmount);
}

Impact

Medium

Tools Used

Manual review

Recommendations

function settleAskMaker(address _offer, uint256 _settledPoints) external {
(OfferInfo memory offerInfo, MakerInfo memory makerInfo, MarketPlaceInfo memory marketPlaceInfo, MarketPlaceStatus status) = getOfferInfo(_offer);
​
-- if (_settledPoints > offerInfo.usedPoints)
++ if (_settledPoints != offerInfo.usedPoints){
revert InvalidPoints();
}
Updates

Lead Judging Commences

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-DeliveryPlace-settleAskTaker-settleAskMaker-partial-settlements

Valid high, in settleAskTaker/settleAskMaker, if the original offer maker performs a partial final settlement, the existing checks [here](https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/DeliveryPlace.sol#L356-L358) and [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/DeliveryPlace.sol#L230-L232) will cause an revert when attempting to complete a full settlement, resulting in their collateral being locked and requiring a rescue from the admin. To note, although examples in the documentation implies settlement in a single click, it is not stated that partial settlements are not allowed, so I believe it is a valid user flow.

Support

FAQs

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