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

ASK Orders are not able to be settled due to wrong checks

vinica_boy

Summary

When a user wants to purchase points in a specific marketplace at a price they are willing to pay, they can create a BID Offer. Conversely, users who are ready to sell their points at that price can place an ASK Order. Once the Token Generation Event (TGE) occurs, it is the responsibility of the sellers to fulfill the deal and transfer the points to the buyer. Sellers are motivated to complete the transaction because they have posted collateral when they created the ASK Order, ensuring their commitment to the deal.
In code terms, Takers need to transfer the points token via settleAskTaker() function in DeliveryPlace.sol. Currently takers are not able to call this function due to wrong permission checks.

Vulnerability Details

As correctly stated in the function's NatSpec, the caller must be the stock authority because the TAKER is responsible for settling the transaction. However, the check on line #361 is incorrect: _msgSender() != offerInfo.authority. In this case, offerInfo.authority refers to the MAKER, who is waiting for the TAKER to deliver the points. The logic should ensure that the correct entity (the TAKER) is being validated for the settlement process.

/**
* @notice Settle ask taker
* @dev caller must be stock authority
* @dev market place status must be AskSettling
* @param _stock stock address
* @param _settledPoints settled points
* @notice _settledPoints must be less than or equal to stock points
*/
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
);
}

Using the protocol test file, I have added another test case which shows the described issue.
NOTE: need to add the following line: import {MarketPlaceStatus} from "../src/interfaces/ISystemConfig.sol";

function testSettleAskTakerPoC() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Bid,
OfferSettleType.Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stock1Addr = GenerateAddress.generateStockAddress(1);
vm.stopPrank();
​
vm.prank(user2);
preMarktes.createTaker(offerAddr, 500);
​
vm.startPrank(user1);
systemConfig.updateMarket("Backpack", address(mockPointToken), 0.01 * 1e18, block.timestamp - 1, 3600);
systemConfig.updateMarketPlaceStatus("Backpack", MarketPlaceStatus.AskSettling);
vm.startPrank(user2);
mockPointToken.approve(address(tokenManager), 10000 * 10 ** 18);
// Next call would revert with Errors.Unauthorized()
deliveryPlace.settleAskTaker(stock1Addr, 500);
}

Impact

This issue is of high severity because it prevents any ASK orders from being settled. As a result, Takers will be unable to complete their transactions, leading to the loss of their collateral. At the same time, Makers will not receive the points tokens they are expecting, which means they will also miss out on the newly airdropped tokens tied to those points. This creates significant financial losses for both Takers and Makers, severely disrupting the functionality of the marketplace.

Tools Used

Manual review, Foundry.

Recommendations

Instead of _msgSender() != offerInfo.authority, the correct check would be _msgSender() != stockInfo.authority.

Updates

Lead Judging Commences

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

Give us feedback!