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

Wrong Id matching leading to accounting errors and unexpected behaviours

turvec

Summary

Wrong Id matching leading to accounting errors

Context
PreMarkets.sol#L117

Vulnerability Details

In the PreMarkets.sol:createOffer() function, the Id used in generating the address for the makerAddr, offerAddr and stockAddr are different from the Id stored in their struct due to already updating the tokenId above but still referring to the new Id in the struct creation causing a mismatch in the entire storage mapping and some orders having the same Id as another e.t.c leading to accounting errors and unexpected behaviours.

​
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
…
​
offerId = offerId + 1;
…
/// @dev update offer info
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
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
});
​

Impact

Wrong Id matching leading to accounting errors

Tools Used

Manaul Review

Recommendations

Update the Id used in their struct creation with the one that was used to generate their addresses as done in other places

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.