Summary
each offer in the protocol includes a status called OfferStatus, which can be one of the following:
* @dev Offer status
* @notice Unknown, Virgin, Ongoing, Canceled, Filled, Settling, Settled
* @param Unknown offer not yet exist.
* @param Virgin offer has been listed, but not one trade.
* @param Ongoing offer has been listed, and already one trade.
* @param Canceled offer has been canceled.
* @param Filled offer has been filled.
* @param Settling offer is settling.
* @param Settled offer has been settled, the last status.
*/
enum OfferStatus {
Unknown,
Virgin,
Ongoing,
Canceled,
Filled,
Settling,
Settled
}
However, the protocol NEVER updates the status to: Ongoing or Filled, which might lead to wrong behaviours and since it is not being updated, it might erroneously pass certain checks in functions and be considered valid, when it should have failed if it had been updated correctly
Vulnerability Details
an example for this vulnerability might be, for example, updating the offerStatus to Ongoing after a user call createTaker function, as it is not Virgin any more.
the createTaker never updates the status, even though it should be updated:
* @notice Create taker
* @param _offer offer address
* @param _points points
*/
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
);
}
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());
address stockAddr = GenerateAddress.generateStockAddress(offerId);
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}
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;
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
);
emit CreateTaker(
_offer,
msg.sender,
stockAddr,
_points,
depositAmount,
tradeTax,
remainingPlatformFee
);
}
therefore, the status will remain Virgin and might pass checks where the status HAS to be Virgin, even though it might not be correct.
Impact
not updating the OfferStatus correctly and in the right functions, might lead to inconsistency and severe problems.
Tools Used
manual review
Recommendations
update the OfferStatus correctly, and implement proper checks on the status in the different functions.
