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

Incorrect Authority Check in `DeliveryPlace::settleAskTaker` Function

pro_king

Description

The DeliveryPlace::settleAskTaker function contains a critical issue where the authority of the stock is incorrectly verified. Specifically, the function checks whether the caller is offerInfo.authority when it should be verifying against stockInfo.authority. This can lead to unauthorized users executing sensitive operations, which could compromise the integrity of the marketplace. The natspec of function says:

@dev caller must be stock authority

but the function give authority to offerInfo.authority.

* @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
);
}
​

Impact

An attacker could potentially exploit this mistake to act as the stock authority, thereby gaining unauthorized control over the settlement process.The function's intended purpose is to ensure that only the correct authority can settle ask orders. The current implementation fails to enforce this, undermining the function's core purpose.

Tools Used

Manual Review

Recommendations

Correct the authority check like below:

if (status == MarketPlaceStatus.AskSettling) {
- if (_msgSender() != offerInfo.authority) {
+ if (_msgSender() != stockInfo.authority) {
revert Errors.Unauthorized();
}
​
Updates

Lead Judging Commences

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