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

Seller can not settle point to buyer

pontifex

Summary

The incorrect authorization prevents users from points settlement via the DeliveryPlace.settleAskTaker function. This causes asset losses if users fail to settle: "If sellers fail to settle in time after the settlement period is over, they will lose their collateral funds and the buyer could claim compensation from your collateral from the smart contract. In the near future, users can set a dynamic collateral deposit." (https://tadle.gitbook.io/tadle/how-tadle-works/features-and-terminologies/settlement-and-collateral-rate)

Vulnerability Details

The DeliveryPlace.settleAskTaker function is used for points settlement and should check if the caller is the stock authority but checks the offer authority instead

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

The authorities in these orders can be different in common case:
PreMarkets.sol:

function createTaker(address _offer, uint256 _points) external payable {
/**
* @dev offer must be virgin
* @dev points must be greater than 0
* @dev total points must be greater than used points plus _points
*/
if (_points == 0x0) {
revert Errors.AmountIsZero();
}
​
>> OfferInfo storage offerInfo = offerInfoMap[_offer];
MakerInfo storage makerInfo = makerInfoMap[offerInfo.maker];
if (offerInfo.offerStatus != OfferStatus.Virgin) {
revert InvalidOfferStatus();
}
<...>
/// @dev update stock info
stockInfoMap[stockAddr] = StockInfo({
id: offerId,
stockStatus: StockStatus.Initialized,
stockType: offerInfo.offerType == OfferType.Ask
? StockType.Bid
: StockType.Ask,
>> authority: _msgSender(),
maker: offerInfo.maker,
preOffer: _offer,
points: _points,
amount: depositAmount,
offer: address(0x0)
});

This way sellers can not settle points and can lose their collateral

Impact

Assets losses due to unexpected behavior.

Tools used

Manual Review

Recommendations

Consider checking if a caller is the stock authority.

Updates

Lead Judging Commences

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