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

Different stocks can have same id in StockInfo struct

matejdb

Summary

Different stocks with different stock addresses can end up with the same id in their StockInfo struct.

Vulnerability Details

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L83C1-L83C31

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L238C1-L253C31

In createOffer() - offerId storage variable that keeps track of the last offer id - is incremented BEFORE being stored on OfferInfo({}) and StockInfo({}) structs as the id of those structs.

In createTaker() - offerId is incremented AFTER being stored as the id of StockInfo({}).

Hence, stocks with different stock addresses (because they are generated with different offerIds) can end up with the same id inside their structs.

Impact

Ids by definition should be unique. This can cause impact off-chain (frontend & backend) of the system rather than on the contract.

We did not detect impact of this vulnerability on the on-chain system.

Still, we believe it constitutes enough for a valid medium because of the implications.

Tools Used

manual review

Recommendations

Increment offerId variable before stock id assignment in createTaker().

offerId = offerId + 1;
​
/// @dev update stock info
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)
});
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.