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

Bid offer cannot be settled by the taker during AskSettling phase

h2134

Summary

Bid offer cannot be settled by the taker during AskSettling phase, due to the incorrect authority check.

Vulnerability Details

When a maker creates a bid offer and the offer is filled by a taker by calling createTaker(), a stock is created and the taker is expected to settle the ask taker during AskSettling phase.

However, the code logic is wrongly implemented and the bid offer cannot be settled by the taker.

function settleAskTaker(address _stock, uint256 _settledPoints) external {
IPerMarkets perMarkets = tadleFactory.getPerMarkets();
StockInfo memory stockInfo = perMarkets.getStockInfo(_stock);
​
(
OfferInfo memory offerInfo,
MakerInfo memory makerInfo,
MarketPlaceInfo memory marketPlaceInfo,
MarketPlaceStatus status
@> ) = getOfferInfo(stockInfo.preOffer);
​
...
​
if (status == MarketPlaceStatus.AskSettling) {
@> if (_msgSender() != offerInfo.authority) {
@> revert Errors.Unauthorized();
@> }
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}
}

As we can see from the code snippet, it requires the authority of the bid offer, i.e the maker to settle the offer, this is obviously wrong because the bid offer should be settled by the authority of the ask stock, i.e the taker instead.

Please run the PoC in PreMarkets.t.sol:

function testAudit_BidOfferCannotBeSettled() public {
// Create Market
address auditMarketPlace = GenerateAddress.generateMarketPlaceAddress("AuditMarket");
vm.startPrank(user1);
systemConfig.createMarketPlace("AuditMarket", false);
systemConfig.updateMarket("AuditMarket", address(mockPointToken), 1e18, block.timestamp + 30 days, 2 days);
vm.stopPrank();
​
address alice = makeAddr("Alice");
deal(address(mockUSDCToken), alice, 1000e18);
​
address bob = makeAddr("Bob");
deal(address(mockUSDCToken), bob, 123.5e18);
​
address aliceOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
​
// Alice creates an Bid offer
vm.startPrank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(CreateOfferParams({
marketPlace: auditMarketPlace,
tokenAddress: address(mockUSDCToken),
points: 1000,
amount: 1000e18,
collateralRate: 12000,
eachTradeTax: 300,
offerType: OfferType.Bid,
offerSettleType: OfferSettleType.Turbo
}));
vm.stopPrank();
​
address bobStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
​
// Bob creates taker
vm.startPrank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(aliceOfferAddr, 100);
vm.stopPrank();
​
// AskSettling
vm.warp(block.timestamp + 31 days);
deal(address(mockPointToken), bob, 100e18);
​
// Bob tries to settle the bid offer but failed
vm.startPrank(bob);
mockPointToken.approve(address(tokenManager), type(uint256).max);
vm.expectRevert(abi.encodeWithSelector(Errors.Unauthorized.selector));
deliveryPlace.settleAskTaker(bobStockAddr, 100);
vm.stopPrank();
}

Impact

Bid offer cannot be settled by the taker, and the taker will lose their collaterals.

Tools Used

Manual Review

Recommendations

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