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

Wrong validation of owner/authority while settling askTaker in deliveryPlace:settleAskTaker()

0xaraj

Summary

Wrong validation of owner/authority while settling askTaker in deliveryPlace:settleAskTaker()

Vulnerability Details

When an user creates a bid/buy offer using preMarket:createOffer(), then sellers can create sell/ask taker using preMarket:createTaker(). Once the marketPlace is updated then seller should call deliveryPlace:settleAskTaker(), which transfers pointsToken from msg.sender to capitalPool which is then added to buyer address.

function settleAskTaker(address _stock, uint256 _settledPoints) external {
...
if (status == MarketPlaceStatus.AskSettling) {
@> if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}
...
uint256 settledPointTokenAmount = marketPlaceInfo.tokenPerPoint * _settledPoints;
ITokenManager tokenManager = tadleFactory.getTokenManager();
if (settledPointTokenAmount > 0) {
@> tokenManager.tillIn(_msgSender(), marketPlaceInfo.tokenAddress, settledPointTokenAmount, true);
​
@> tokenManager.addTokenBalance(
TokenBalanceType.PointToken, offerInfo.authority, makerInfo.tokenAddress, settledPointTokenAmount
);
}
...
}

Now the problem is, settleAskTaker() verifies the authority/owner of offerInfo(ie offerInfo.authority) with msg.sender instead of verifying msg.sender with stockInfo.authority

OfferInfo.authority is the buyer's address whom we are adding pointsToken using tokenManager:addTokenBalance(). stockInfo.authority is seller's address who is transfering pointsToken to capitalPool using tokenManager:tillIn()

//Here is PoC which shows above situation

function test_sellerCanNotCall() public {
//User created a bid offer
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 1000e18, 10000, 300, OfferType.Bid, OfferSettleType.Turbo
)
);
vm.stopPrank();
​
//User2 created ask/sell offer
vm.prank(user2);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 1000);
​
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 1e18, block.timestamp - 1, 3600);
​
//User2 ie seller trying to settle askTaker but reverted due to unauthorized
vm.startPrank(user2);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.expectRevert();
deliveryPlace.settleAskTaker(stock1Addr, 1000);
vm.stopPrank();
}

Impact

Seller will not be able to settle the askTaker

Tools Used

Manual Review

Recommendations

Use stockInfo.authority instead of offerInfo.authority in deliveryPlace:settleAskTaker()

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