Tadle

Tadle
DeFiFoundry
27,750 USDC
View results
Submission Details
Severity: medium
Invalid

Incorrect Points Validation in `createTaker` Function

Github

Summary

The createTaker function in the PreMarktes contract is responsible for allowing users to create a "taker" for an existing offer. The function ensures that the taker complies with the parameters and restrictions of the offer, such as the offer's status, the points to be utilized, and other financial constraints. However, there is a critical issue in the validation logic for the points, which could lead to erroneous behavior, such as accepting an invalid number of points for a taker.

Vulnerability Details

The intention is to ensure that the total points requested by the taker (_points) combined with the points already used in the offer (offerInfo.usedPoints) does not exceed the total points available in the offer (offerInfo.points). However, the current code uses the < operator to check this condition. This comparison will fail to reject cases where the sum of _points and offerInfo.usedPoints equals offerInfo.points, which is contrary to the intention stated in the comment that points must be greater.

See the following code:

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();
}
if (offerInfo.points < _points + offerInfo.usedPoints) {
revert NotEnoughPoints(
offerInfo.points,
offerInfo.usedPoints,
_points
);
}
/// @dev market place must be online
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
{
MarketPlaceInfo memory marketPlaceInfo = systemConfig
.getMarketPlaceInfo(makerInfo.marketPlace);
marketPlaceInfo.checkMarketPlaceStatus(
block.timestamp,
MarketPlaceStatus.Online
);
}
ReferralInfo memory referralInfo = systemConfig.getReferralInfo(
_msgSender()
);
uint256 platformFeeRate = systemConfig.getPlatformFeeRate(_msgSender());
/// @dev generate stock address
address stockAddr = GenerateAddress.generateStockAddress(offerId);
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}
/// @dev Transfer token from user to capital pool as collateral
uint256 depositAmount = _points.mulDiv(
offerInfo.amount,
offerInfo.points,
Math.Rounding.Ceil
);
uint256 platformFee = depositAmount.mulDiv(
platformFeeRate,
Constants.PLATFORM_FEE_DECIMAL_SCALER
);
uint256 tradeTax = depositAmount.mulDiv(
makerInfo.eachTradeTax,
Constants.EACH_TRADE_TAX_DECIMAL_SCALER
);
ITokenManager tokenManager = tadleFactory.getTokenManager();
_depositTokenWhenCreateTaker(
platformFee,
depositAmount,
tradeTax,
makerInfo,
offerInfo,
tokenManager
);
offerInfo.usedPoints = offerInfo.usedPoints + _points;
/// @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)
});
offerId = offerId + 1;
uint256 remainingPlatformFee = _updateReferralBonus(
platformFee,
depositAmount,
stockAddr,
makerInfo,
referralInfo,
tokenManager
);
makerInfo.platformFee = makerInfo.platformFee + remainingPlatformFee;
_updateTokenBalanceWhenCreateTaker(
_offer,
tradeTax,
depositAmount,
offerInfo,
makerInfo,
tokenManager
);
/// @dev emit CreateTaker
emit CreateTaker(
_offer,
msg.sender,
stockAddr,
_points,
depositAmount,
tradeTax,
remainingPlatformFee
);
}

Impact

This issue allows the function to accept cases where all remaining points in the offer are consumed by the taker, which may not be the intended behavior, especially if the platform's logic requires some points to remain. This could lead to unexpected behavior in the marketplace, potentially disrupting the matching process or causing issues with the offer lifecycle.

Tools Used

Manual Review

Recommendations

The comparison should be adjusted to use the <= operator instead of <, ensuring that the function rejects cases where _points + offerInfo.usedPoints equals or exceeds offerInfo.points

Updates

Lead Judging Commences

0xnevi Lead Judge
12 months ago
0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.