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

Premature offerId Increment in PreMarktes::createOffer Function

josh4324

Summary

The createOffer function is responsible for creating new offers and updating various mappings ( offerInfoMap, stockInfoMap) with the relevant offerId. However, a critical issue has been identified where the offerId is incremented prematurely, before the updates to these mappings are finalized. This early increment causes incorrect offerId values to be stored in the mappings, leading to data inconsistencies, potential loss of offer tracking, and operational errors within the system.

Vulnerability Details

Within the createOffer function, the offerId is incremented too early in the process, before the offerInfoMap, and stockInfoMap are updated. This premature increment means that these mappings are updated with an incorrect offerId, which does not correspond to the actual offer being created.

function createOffer(CreateOfferParams calldata params) external payable {
/**
* @dev points and amount must be greater than 0
* @dev eachTradeTax must be less than 100%, decimal scaler is 10000
* @dev collateralRate must be more than 100%, decimal scaler is 10000
*/
if (params.points == 0x0 || params.amount == 0x0) {
revert Errors.AmountIsZero();
}
​
if (params.eachTradeTax > Constants.EACH_TRADE_TAX_DECIMAL_SCALER) {
revert InvalidEachTradeTaxRate();
}
​
if (params.collateralRate < Constants.COLLATERAL_RATE_DECIMAL_SCALER) {
revert InvalidCollateralRate();
}
​
/// @dev market place must be online
ISystemConfig systemConfig = tadleFactory.getSystemConfig();
MarketPlaceInfo memory marketPlaceInfo = systemConfig.getMarketPlaceInfo(params.marketPlace);
marketPlaceInfo.checkMarketPlaceStatus(block.timestamp, MarketPlaceStatus.Online);
​
/// @dev generate address for maker, offer, stock.
/// correct offerId used to generate the Addresses below
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
​
if (makerInfoMap[makerAddr].authority != address(0x0)) {
revert MakerAlreadyExist();
}
​
if (offerInfoMap[offerAddr].authority != address(0x0)) {
revert OfferAlreadyExist();
}
​
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}
​
// Premature offerId increment
@> offerId = offerId + 1;
​
{
/// @dev transfer collateral from _msgSender() to capital pool
uint256 transferAmount = OfferLibraries.getDepositAmount(
params.offerType, params.collateralRate, params.amount, true, Math.Rounding.Ceil
);
​
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.tillIn{value: msg.value}(_msgSender(), params.tokenAddress, transferAmount, false);
}
​
/// @dev update maker info
// updated with wrong offerId
makerInfoMap[makerAddr] = MakerInfo({
offerSettleType: params.offerSettleType,
authority: _msgSender(),
marketPlace: params.marketPlace,
tokenAddress: params.tokenAddress,
originOffer: offerAddr,
platformFee: 0,
eachTradeTax: params.eachTradeTax
});
​
/// @dev update offer info
// updated with wrong offerId
offerInfoMap[offerAddr] = OfferInfo({
@> id: offerId,
authority: _msgSender(),
maker: makerAddr,
offerStatus: OfferStatus.Virgin,
offerType: params.offerType,
points: params.points,
amount: params.amount,
collateralRate: params.collateralRate,
abortOfferStatus: AbortOfferStatus.Initialized,
usedPoints: 0,
tradeTax: 0,
settledPoints: 0,
settledPointTokenAmount: 0,
settledCollateralAmount: 0
});
​
/// @dev update stock info
// updated with wrong offerId
stockInfoMap[stockAddr] = StockInfo({
@> id: offerId,
stockStatus: StockStatus.Initialized,
stockType: params.offerType == OfferType.Ask ? StockType.Bid : StockType.Ask,
authority: _msgSender(),
maker: makerAddr,
preOffer: address(0x0),
offer: offerAddr,
points: params.points,
amount: params.amount
});
​
emit CreateOffer(
offerAddr, makerAddr, stockAddr, params.marketPlace, _msgSender(), params.points, params.amount
);
}

Impact

The incorrect offerId stored in these mappings results in data inconsistencies across the system. The mappings may reference non-existent or incorrect offers, leading to failures in offer tracking, incorrect data retrieval, and potential errors in subsequent operations that rely on accurate offer information.

Tools Used

Manual Review

Recommendations

The increment of the offerId should be done at the end of the function or after the mappings has been updated.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-createOffer-offerId-increment-after

I believe this is valid low severity, although there is inconsistency here when using the correct `offerId` for assigning offerIds and generating the unique addresses as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L67-L69), this is purely an accounting error for offerIds. If we generate the offerId using current `offerId - 1`, the appropriate listing/taker orders can still be created against those offers.

Support

FAQs

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