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

Incorrect Offer ID Increment Causes Offer Creation Failure

krisp

Summary

The PreMarket#createOffer function is currently unable to create more than one offer, which renders the project unusable.

Vulnerability Details

The PreMarket#createOffer function is designed to create virtual offers, which are then added to the market using the PreMarket#listOffer function. However, there is a flaw in the current design that prevents the creation of additional offers after the first one. Here’s a detailed explanation of how this issue arises:

To create an offer, random addresses for the maker, offer, and stock are generated using the offerId state variable as follows:

address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);

Before creating the offer and setting up the makerInfoMap, offerInfoMap, and stockInfoMap mappings, the offerId state variable is incremented:

offerId = offerId + 1;

However, this incremented offerId is incorrectly passed as the offerId in the offer creation process:

PreMarkets#createOffer

/// @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
});

PreMarkets#createOffer

/// @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
});

Due to this incrementing issue, the next call to PreMarket#createOffer will fail. This is because the function will revert with the following checks: the offerAddr is generated based on the incremented offerId, but offerId has already been set to the previous offer:

...
/// @dev generate address for maker, offer, stock.
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
​
...
​
👉 if (offerInfoMap[offerAddr].authority != address(0x0)) {
revert OfferAlreadyExist();
}

Impact

The malfunction of one of the main project functionalities means users cannot create offers, rendering the system unusable.

Tools Used

manual code review

Recommendations

Increment the offerId after setting the mappings, rather than before.

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!