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

Call to settleAskTaker() will fail every time due to wrong authority check.

itsabinashb

Summary

Call to settleAskTaker() will fail every time due to wrong authority check.

Vulnerability Details

The DeliveryPlace::settleAskTaker() is suppoesed to be called by takers of bid offer to release pointToken, amount of point token is caculated by multiplying the marketPlaceInfo.tokenPerPoint with the amount of point the taker wants to settle.
But taker's call to this function will revert every time because of incorrect authority check, the check is done in the function like this:

if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}

Here the function expecting the offer's owner as caller, but offer's owner will never call this function, this function is supposed to call by the stock owner i.e taker. Only then point tokens will be deducted from the taker.

POC

Run this test in PreMarkets.t.sol contract, just add an address as user4 and give him enough tokens.

function test_settlingRevert() public {
​
//@audit User creating a Bid offer, to buy 1000 point
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
vm.stopPrank();
​
//@audit User4 created a stock to sell 500 point to user's Bid offer
vm.startPrank(user4);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
​
//@audit updateMarket() is called to set the timestamp in 'settlementPeriod' i.e tge was done
// & we are in now settlementPeriod
vm.startPrank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
//@audit updating the marketPlaceStatus to AskSettling
systemConfig.updateMarketPlaceStatus("Backpack", MarketPlaceStatus.AskSettling);
vm.stopPrank();
​
//@audit Now the user came & closed the Bid offer
vm.prank(user);
deliveryPlace.closeBidOffer(offerAddr);
vm.startPrank(user4);
//@audit user4 tried to settle his Ask type stock so that he can sell points to the user
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
//@audit But it is reverting
vm.expectRevert(Errors.Unauthorized.selector);
deliveryPlace.settleAskTaker(stock1Addr, 500);
vm.stopPrank();
}
​

Tool Used

Manual review, Foundry

Recommendation

- if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}
​
+ if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}

Related Links

Updates

Lead Judging Commences

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

finding-PreMarkets-settleAskTaker-wrong-stock-authority

Valid high severity, when taker offers are created pointing to a `offer`, the relevant `stockInfoMap` offers are created with the owner of the offer aka `authority`, set as the creater of the offer, as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L245). Because of the wrong check within settleAskTaker, it will permanently DoS the final settlement functionality for taker offers for the maker that listed the original offer, essentially bricking the whole functionality of the market i.e. maker will always get refunded the original collateral, and takers will never be able to transact the original points put up by the maker. This occurs regardless of market mode.

Support

FAQs

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