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

offerId not incremented correctly.

0xgenaudits

Summary

The current implementation of offer ID management within the smart contract can lead to conflicts during
the creation of new offers. Specifically, if the offerId is not updated correctly after being stored,
The offerId should be incremented after being used to avoid potential conflicts and ensure the uniqueness of each offer.

Vulnerability Details

The problem is that offerId is incremented before it is stored in offerInfoMap[offerAddr] so when offerId
is stored it will be stored as offerId + 1.

function createOffer(CreateOfferParams calldata params) external payable {
@>>> // offerId -> 0
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
​
// ....
@>>> // offerId = offerId: 0 + 1;
offerId = offerId + 1;
​
// ....
offerInfoMap[offerAddr] = OfferInfo({
@>>> id: offerId, // -> 1
// ...
});
​
stockInfoMap[stockAddr] = StockInfo({
@>>> id: offerId, // -> 1
// ...
});
}

Impact

If the offerId is not incremented correctly, there can be several negative impacts:

Tools Used

Recommendations

Ensure that offerId is incremented after it is stored in the offerInfoMap.

function createOffer(CreateOfferParams calldata params) external payable {
// ....
offerInfoMap[offerAddr] = OfferInfo({
id: offerId, // -> 0
// ...
});
​
stockInfoMap[stockAddr] = StockInfo({
id: offerId, // -> 0
// ...
});
​
offerId = offerId + 1;
}
Updates

Lead Judging Commences

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