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

Incorrect id is stored in OfferInfo and StockInfo when offer is created

bkweb3

Summary

A new offer can be created via PreMarkets.createOffer function. When an offer is being created, 3 addresses are generated from offerId - makerAddr, offerAddrand stockAddr. Those addresses are used as keys in mappings to store MakerInfo, OfferInfo and StockInfo.

The issue is that when those addresses are made and OfferInfo and StockInfo is stored, two different offerIds are used.

Vulnerability Details

makerAddr, offerAddr and stockAddr are generated as follows

/// @dev generate address for maker, offer, stock.
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();
}
​
offerId = offerId + 1;

Notice that offerId is increased right after the addresses are generated.

Later on the offerId that got increased by 1 is used as id inside OfferInfo and StockInfo

/// @dev update offer info
offerInfoMap[offerAddr] = OfferInfo({
id: offerId,
...
});
​
/// @dev update stock info
stockInfoMap[stockAddr] = StockInfo({
id: offerId,
...
});

This means that the id that is stored in OfferInfo and StockInfo is incorrect. The current createOffer flow will always cause offerId to be used in key, but offerId + 1 will be stored as id.

Impact

Incorrect id is stored in OfferInfo and StockInfo

Tools Used

Manual Review

Recommendations

Make sure that the offerId is increased after OfferInfo and StockInfo is stored.

- 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
makerInfoMap[makerAddr] = MakerInfo({
...
});
​
/// @dev update offer info
offerInfoMap[offerAddr] = OfferInfo({
...
});
​
/// @dev update stock info
stockInfoMap[stockAddr] = StockInfo({
...
});
​
+ offerId = offerId + 1;
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year 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.