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

Incorrect Increment of `offerId` Causes Misassignment of `offerInfoMap` and `stockInfoMap`, Leading to Data Inconsistencies

pina

Summary

When the owner of an offer calls PreMarkets::createOffer, the offerId is used to generate addresses for the maker, offer, and stock. However, the offerId is incremented before updating the offerInfoMap and stockInfoMap, leading to the association of incorrect data with the offerId.

Vulnerability Details

In the PreMarkets::createOffer function, the offerId is used to generate addresses through the GenerateAddress library. However, the offerId is incremented before the offer and stock information is assigned to the respective maps (offerInfoMap and stockInfoMap). This results in the assignment of these data structures to the wrong offerId, causing data inconsistencies:

/// @dev generate address for maker, offer, stock.
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
​
if (makerInfoMap[makerAddr].authority != address(0x0)) {
revert MakerAlreadyExist();
}
​
if (offerInfoMap[offerAddr].authority != address(0x0)) {
revert OfferAlreadyExist();
}
​
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}
​
offerId = offerId + 1; // <== ISSUE HERE
...
...
...
/// @dev update offer info
offerInfoMap[offerAddr] = OfferInfo({
id: offerId, // <== SET WITH DIFFERENT `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, // <== SET WITH DIFFERENT `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

The incorrect increment of offerId results in the assignment of wrong information to offerInfoMap and stockInfoMap, leading to potential data inconsistencies. This could cause issues in identifying and managing offers and stocks correctly within the protocol, possibly resulting in faulty transactions, mismanagement of assets, and difficulty in tracking offer data.

Tools Used

  • Manual Code Review

Recommendations

To resolve this issue, increment the offerId before generating the addresses and performing checks, ensuring that the correct offerId is used for updating the offerInfoMap and stockInfoMap:

+ offerId = offerId + 1;
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
​
if (makerInfoMap[makerAddr].authority != address(0x0)) {
revert MakerAlreadyExist(); // i: if already exist DoS
}
​
if (offerInfoMap[offerAddr].authority != address(0x0)) {
revert OfferAlreadyExist();
}
​
if (stockInfoMap[stockAddr].authority != address(0x0)) {
revert StockAlreadyExist();
}
​
- offerId = offerId + 1;
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.