Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Flawed Flow in createOffer for BID

josh4324

Summary

The createOffer function for BID offers is supposed to allow users to place and match orders for buying point tokens. However, this flow is not functioning as intended, unlike the createOffer for ASK offers, which allows the Maker to createOffer for selling point tokens. There is a problem with the createOffer for BID and thesettleAskTaker function, which is supposed to complete the settlement process after the Token Generation Event (TGE). The issue lies in the fact that settleAskTaker is only callable by the Maker, who is incorrectly expected to send the point tokens. This misalignment results in the function potentially reverting if the Maker does not have the point tokens, which is likely, given that the tokens should be sent by the Taker, and loss of the point tokens if the Maker has the points tokens.

Vulnerability Details

The settleAskTaker function is designed to be called by the Maker to finalize the settlement of point tokens after the TGE. However, in the context of a BID offer, the Taker is supposed to provide the point tokens. The current implementation mistakenly expects the Maker to hold and transfer the point tokens, which contradicts the intended flow where the Maker is the buyer, not the seller.

Issues

  1. The createOffer for BID makes the Maker the owner of the stock authority.

  2. The settleAskTaker can only be called by stock authority or contract owner

  3. If Maker calls the settleAskTaker function, the transaction will revert or the maker will lose point tokens

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 (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus();
}
​
if (marketPlaceInfo.fixedratio) {
revert FixedRatioUnsupported();
}
if (stockInfo.stockType == StockType.Bid) {
revert InvalidStockType();
}
if (_settledPoints > stockInfo.points) {
revert InvalidPoints();
}
​
if (status == MarketPlaceStatus.AskSettling) {
@> if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}
} else {
if (_msgSender() != owner()) {
revert Errors.Unauthorized();
}
if (_settledPoints > 0) {
revert InvalidPoints();
}
}
​
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
);
}
​
uint256 collateralFee = OfferLibraries.getDepositAmount(
offerInfo.offerType, offerInfo.collateralRate, stockInfo.amount, false, Math.Rounding.Floor
);
​
if (_settledPoints == stockInfo.points) {
tokenManager.addTokenBalance(
TokenBalanceType.RemainingCash, _msgSender(), makerInfo.tokenAddress, collateralFee
);
} else {
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund, offerInfo.authority, makerInfo.tokenAddress, collateralFee
);
}
​
perMarkets.settleAskTaker(stockInfo.preOffer, _stock, _settledPoints, settledPointTokenAmount);
​
emit SettleAskTaker(
makerInfo.marketPlace,
offerInfo.maker,
_stock,
stockInfo.preOffer,
_msgSender(),
_settledPoints,
settledPointTokenAmount,
collateralFee
);
}

POC

function test_SellFlow() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 1000 * 1e18, 12000, 300, OfferType.Bid, OfferSettleType.Turbo
)
);
vm.stopPrank();
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
​
vm.startPrank(user3);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
​
OfferInfo memory info = preMarktes.getOfferInfo(offerAddr);
​
vm.prank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 2 * 1e18, block.timestamp - 1, 3600);
​
// transaction will revert with Unauthorized()
vm.startPrank(user3);
mockPointToken.approve(address(tokenManager), 1000 * 10 ** 18);
deliveryPlace.settleAskTaker(stock1Addr, 1000);
vm.stopPrank();
}

Impact

Users will not be able to complete BID offers.

Tools Used

Manual Review

Recommendations

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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.