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

Different stocks/offers can be created with the same id

h2134

Summary

Different stocks/offers can be created with the same id.

Vulnerability Details

According to NatSpec of struct StockInfo, id should be the unique id of stock.

@param id the unique id of stock.

However, different stocks can be created with the same id.

When an offer is created, offerId is incremented by and is assigned to the stock info which is created along with the offer. At this time, offer and stock have the same id.

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

The stock can be filled and a sub stock will be created, then `offerId` is incremented by .

/// @dev update stock info
stockInfoMap[stockAddr] = StockInfo({
@> id: offerId,
stockStatus: StockStatus.Initialized,
stockType: offerInfo.offerType == OfferType.Ask
? StockType.Bid
: StockType.Ask,
authority: _msgSender(),
maker: offerInfo.maker,
preOffer: _offer,
points: _points,
amount: depositAmount,
offer: address(0x0)
});
​
@> offerId = offerId + 1;

This means the origin stock and the sub stock shares the same id, and this breaks the promise that id shoud be unique across different stocks.

Likewise, when user list the sub stock, a sub offer is created, and this sub offer will use the same id as the sub stock. This means the origin offer and the sub offer have the same id.

/// @dev update offer info
offerInfoMap[offerAddr] = OfferInfo({
@> id: stockInfo.id,
authority: _msgSender(),
maker: offerInfo.maker,
offerStatus: OfferStatus.Virgin,
offerType: offerInfo.offerType,
abortOfferStatus: AbortOfferStatus.Initialized,
points: stockInfo.points,
amount: _amount,
collateralRate: _collateralRate,
usedPoints: 0,
tradeTax: 0,
settledPoints: 0,
settledPointTokenAmount: 0,
settledCollateralAmount: 0
});

Please run the PoC in PreMarkets.t.sol to verify:

function testAudit_OfferId() public {
// Create Market
address auditMarketPlace = GenerateAddress.generateMarketPlaceAddress("AuditMarket");
vm.startPrank(user1);
systemConfig.createMarketPlace("AuditMarket", false);
systemConfig.updateMarket("AuditMarket", address(mockPointToken), 0.5e18, block.timestamp + 30 days, 2 days);
vm.stopPrank();
​
address alice = makeAddr("Alice");
deal(address(mockUSDCToken), alice, 1200e18);
​
address bob = makeAddr("Bob");
deal(address(mockUSDCToken), bob, 1005e18);
​
address aliceStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
address aliceOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
​
// Alice creates an Ask offer
vm.startPrank(alice);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createOffer(CreateOfferParams({
marketPlace: auditMarketPlace,
tokenAddress: address(mockUSDCToken),
points: 1000,
amount: 1000e18,
collateralRate: 12000,
eachTradeTax: 0,
offerType: OfferType.Ask,
offerSettleType: OfferSettleType.Turbo
}));
vm.stopPrank();
​
address bobStockAddr = GenerateAddress.generateStockAddress(preMarktes.offerId());
address bobOfferAddr = GenerateAddress.generateOfferAddress(preMarktes.offerId());
​
// Bob creates taker and lists a sub offer
vm.startPrank(bob);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(aliceOfferAddr, 1000);
preMarktes.listOffer(bobStockAddr, 100e18, 12000);
vm.stopPrank();
​
StockInfo memory aliceStockInfo = preMarktes.getStockInfo(aliceStockAddr);
StockInfo memory bobStockInfo = preMarktes.getStockInfo(bobStockAddr);
​
assertEq(aliceStockInfo.id, bobStockInfo.id);
​
OfferInfo memory aliceOfferInfo = preMarktes.getOfferInfo(aliceOfferAddr);
OfferInfo memory bobOfferInfo = preMarktes.getOfferInfo(bobOfferAddr);
​
assertEq(aliceOfferInfo.id, bobOfferInfo.id);
}

Impact

Different stocks/offers have the same id.

Tools Used

Manual Review

Recommendations

When create offer, assign offerId to stock/offer before incrementing offerId by .

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.