Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

`offerId` is not correctly updated in PreMarkets::createOffer()

0xaraj

Summary

offerId is not correctly updated in PreMarkets::createOffer()

Vulnerability Details

When a user creates an offer using createOffer(), then it generates maker/stock/offer address using offerId as input. Now the problem is, it updates the offerId before using it in maker/stock/offer struct, which leads that ids of the structs are set to the next offerId ie other than that is used to generate their addresses

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

Now this will create issue down the line while creating taker or listing offer or again creating offer
//Lets see how this will create issue while creating taker

  1. Suppose a user created offer then stock/maker/offer address will be generated with offerId = 0 but id of all 3 structs will be set to 1(due to early updation of offerId)

  2. Another user created taker for that offer then createTaker() will generate stock address with offerId = 1 & will set id = offerId = 1 in stock struct, then it will increase the offerId = 2. Here problem is we have 2 stock struct which has id = 1(one that was created in step 1 & second in this step 2)

function createTaker(address _offer, uint256 _points) external payable {
....
/// @dev generate stock address
address stockAddr = GenerateAddress.generateStockAddress(offerId);
​
....
/// @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;
...
}

//Here is PoC

function test_WrongUpdateOfOfferId() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stockAddr = GenerateAddress.generateStockAddress(0);
StockInfo memory stockInfo = preMarktes.getStockInfo(stockAddr);
//Before creating taker, stock.id of offerId = 0 is 1
assertEq(stockInfo.id, 1);
​
preMarktes.createTaker(offerAddr, 500);
address stock1Addr = GenerateAddress.generateStockAddress(1);
StockInfo memory stock1Info = preMarktes.getStockInfo(stock1Addr);
//After creating taker, stock.id of offerId = 1 is also 1
assertEq(stock1Info.id, 1);
}

Impact

Multiple structs with same id will be created

Tools Used

Manual Review

Recommendations

Either update the offerId before generating addresses or after setting all 3 structs in createOffer()

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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.