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

Incorrect access control in function `DeliverPlace::settleAskTaker` prevents ask takers from settling offers

robertodf99

Summary

The DeliverPlace::settleAskTaker function is designed for ask takers to finalize their transactions by transferring points and retrieving their collateral. However, due to a flawed access control mechanism, only the bid maker (offer authority) is permitted to call this function, preventing ask takers from completing their settlements.

Vulnerability Details

The incorrect access check in the code is as follows:

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

Impact

See following PoC for a detailed example:

function test_create_bid_offer_turbo_eth() public {
vm.startPrank(user);
​
preMarktes.createOffer{value: 0.01 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
​
vm.stopPrank();
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
preMarktes.createTaker{value: 0.006175 * 1e18}(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
​
vm.prank(user1);
systemConfig.updateMarket(
"Backpack",
address(mockPointToken),
0.01 * 1e18,
block.timestamp - 1,
3600
);
​
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
​
// address(this) as the authority of the stock calls settleAskTaker
vm.expectRevert(Errors.Unauthorized.selector); // Import Errors from Utils
deliveryPlace.settleAskTaker(stock1Addr, 500);
}

Tools Used

Manual review.

Recommendations

Implement correct access control:

if (status == MarketPlaceStatus.AskSettling) {
if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
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.