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

DOS for partially settled offers

audinarey

Summary

The DeliveryPlace::settleAskMaker(...) allows a user to enter the amount points they want to settle within the limit of usedPointof their _offer. If a user decides to settle in bits they will not be able to settle their outstanding usedPoints because DeliveryPlace::settleAskMaker(...)will revert.

Vulnerability Details

function settleAskMaker(address _offer, uint256 _settledPoints) external {
(
OfferInfo memory offerInfo,
MakerInfo memory makerInfo,
MarketPlaceInfo memory marketPlaceInfo,
MarketPlaceStatus status
) = getOfferInfo(_offer);
​
........
if (
offerInfo.offerStatus != OfferStatus.Virgin &&
offerInfo.offerStatus != OfferStatus.Canceled
) {
@> revert InvalidOfferStatus();
}
...........
​
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
@> perMarkets.settledAskOffer(
_offer,
_settledPoints,
settledPointTokenAmount
);
​
emit SettleAskMaker(
makerInfo.marketPlace,
offerInfo.maker,
_offer,
_msgSender(),
_settledPoints,
settledPointTokenAmount,
makerRefundAmount
);
}

As shown in the code snippet above, DeliveryPlace::settleAskMaker(...) makes an internal call to PreMarkets::settleAskOffer(...) to update the offer's settlement for ASKs. As shown below, the offerInfo.offerStatusis updated to OfferStatus.Settled so when settledAskOfferis called.

function settledAskOffer(
address _offer,
uint256 _settledPoints,
uint256 _settledPointTokenAmount
) external onlyDeliveryPlace(tadleFactory, _msgSender()) {
OfferInfo storage offerInfo = offerInfoMap[_offer];
@> offerInfo.settledPoints = _settledPoints;
offerInfo.settledPointTokenAmount = _settledPointTokenAmount;
@> offerInfo.offerStatus = OfferStatus.Settled;
​
emit SettledAskOffer(_offer, _settledPoints, _settledPointTokenAmount);
}

  • Alice has 1000 point to settle

  • she first calls DeliveryPlace::settleAskMaker(...)with 500 points

  • she attempts to call DeliveryPlace::settleAskMaker(...)with the remaining 500 points but the transaction revert.

  • Admin has access to the function for all users and calls DeliveryPlace::settleAskMaker(...)on behalf of Alice but the function reverts becuase the offerInfo.offerStatus is neither Cancelled nor Virgin

** CODED POC**

Add the test case below to the PreMarkets.t.solfile and run forge test --mt test_ask_offer_settle_revert -vvv

function test_ask_offer_settle_revert() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, // marketPlace;
address(mockUSDCToken), // tokenAddress;
1000, // points;
0.01 * 1e18, // amount;
12000, // collateralRate;
300, // eachTradeTax;
OfferType.Ask, // ASK
OfferSettleType.Turbo // Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000);
​
address offer1Addr = GenerateAddress.generateOfferAddress(1);
preMarktes.closeOffer(stock1Addr, offer1Addr);
preMarktes.relistOffer(stock1Addr, offer1Addr);
​
vm.stopPrank();
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
deliveryPlace.settleAskMaker(offerAddr, 250);
vm.expectRevert();
deliveryPlace.settleAskMaker(offerAddr, 250);
deliveryPlace.closeBidTaker(stock1Addr);
vm.stopPrank();
}

Impact

DoS on Delivery market (could also lead to possible stuck funds)

Tools Used

Manual review

Recommendations

Prevent users from entering the amount of point they want to settle. Instead they should settle all available offerInfo.usedPointsas shown below when settleAskMaker(...)is called.

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