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

Inconsistent `offerId` in `createOffer`

vineyard

Summary

The offerIdstored during createOfferis inconsistent with offerIdbeing used for address generation.

Vulnerability Details

In the createOffer function of the PreMarkets contract, there is an inconsistency on the stored offerId is used. The function uses the current offerId to generate unique addresses for the maker, offer, and stock.

/// @dev generate address for maker, offer, stock.
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);

However, it then increments the offerId before storing it in the OfferInfo struct.

offerId = offerId + 1;

This results in a mismatch between the offerId used for address generation and the offerId stored in the offer/stock information.

// OfferInfo stored with incremented offerId
offerInfoMap[offerAddr] = OfferInfo({
id: offerId, // This is now different from the id used for address generation
...
});
​
// stockInfoMap stored with incremented offerId
stockInfoMap[stockAddr] = StockInfo({
id: offerId,
...
});

This can be tested with following test:

function test_inconsistent_offerid() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stockAddr = GenerateAddress.generateStockAddress(0);
​
OfferInfo memory offerInfo = preMarktes.getOfferInfo(offerAddr);
StockInfo memory stockInfo = preMarktes.getStockInfo(stockAddr);
​
assertEq(offerInfo.id, 1);
assertEq(stockInfo.id, 1);
}

Impact

Mismatched IDs between offer/stock addresses and stored offer/stock information.

Tools Used

Manual

Recommendations

Modify the createOffer function to use the same offerId for both address generation and storage.

Updates

Lead Judging Commences

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

Give us feedback!