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

[H-3] `offerId` stored in `offerInfoMap` and `stockInfoMap` is 1 more than `orderId` used to generate the respective addresses

karanel

Relevant Links

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L39-L157

Summary

offerId stored in offerInfoMap and stockInfoMap is 1 more than orderId used to generate the respective addresses. This can cause incorrect orders or stocks to be matched when calling any of PreMarkets::createTaker, PreMarkets::listOffer, PreMarkets::closeOffer, PreMarkets::relistOffer, PreMarkets::abortAskOffer, PreMarkets::abortBidTaker, PreMarkets::settledAskOffer and PreMarkets::settleAskTaker functions.

Vulnerability Details

Here as it can be seen in PreMarkets::createOffer function, firstly the orderId is used to generate an address by using the keccak256 hash, and then increased.

address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
...
offerId = offerId + 1; <<<< offerId increased
...
/// @dev update offer info
offerInfoMap[offerAddr] = OfferInfo({ <<<< old offerId address
id: offerId, <<<< new 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({ <<<< old offerId address
id: offerId, <<<< new 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

Likelihood: High
Impact: Med/High - This would cause inconsistencies in offerIds while interacting with the protocol

Overall severity is High.

Tools Used

Manual Review

Recommendations

Update offerId before generating the addresses

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!