Tadle

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

Incorrect offerId used in offerInfoMap and stockInfoMap in createOffer

Summary

function createOfferis used to create a offer in PreMarkets.sol. It also creates three mappings entry(makerInfoMap, offerInfoMap and stockInfoMap) during creation of offer. The wrong offerIdis used in the struct of offerInfoMapand stockInfoMap.

Vulnerability Details

In createOffer, offerIdis used to determine makerAddr, stockAddrand offerAddr. Once, address is determined, the offerIdis incremented as shown below. After incrementing, while filling the struct data for offerInfoMapand stockInfoMap, it is using incremented offerIdinstead of the offerIdthrough which addresses are generated.

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

function createOffer(CreateOfferParams calldata params) external payable {
....
/// @dev generate address for maker, offer, stock.
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
.....
offerId = offerId + 1; ///// OFFERID is incremented
......
offerInfoMap[offerAddr] = OfferInfo({
id: offerId, //// ISSUE: USES WRONG OFFER ID
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, //// ISSUE: USES WRONG OFFER ID
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 data being filled in offerInfoMapand stockInfoMap.

Tools Used

Manual review

Recommendations

Update the code as follows:

function createOffer(CreateOfferParams calldata params) external payable {
....
/// @dev generate address for maker, offer, stock.
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
.....
offerId = offerId + 1; ///// OFFERID is incremented
......
offerInfoMap[offerAddr] = OfferInfo({
id: offerId - 1, //// ISSUE FIXED
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 - 1, //// ISSUE FIXED
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
});
.....
}
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.