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

Denial of Service (DoS) in Offer Listing

salem

Summary

The listOfferfucntion in PreMarktes contract that can lead to a Denial of Service (DoS) condition, effectively preventing users from listing offers beyond the first one. This vulnerability stems from an inconsistency in the offer ID management between offer creation and listing processes.

Vulnerability Details

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

  • When createOffer() is called, it uses the current offerId (e.g. offerId= 0) to generate addresses

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

  • After generating these addresses, the function increments offerId.

offerId = offerId + 1;
//PreMarkets.sol
//createOffer() function
66: /// @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();
}
​
83: offerId = offerId + 1;

  • This means that for the first offer, addresses are generated with offerId = 0, but the offer is stored with id = 1.

  • For the second offer, addresses are generated with offerId = 1, but the offer is stored with id = 2.

  • When listOffer() is called, it generates the offer address using stockInfo.id:

address offerAddr = GenerateAddress.generateOfferAddress(stockInfo.id);

  • stockInfo.id is set to the offerId value at the time the stock was created, which is the incremented value. This means for the first offer's stock, stockInfo.id = 1, not 0.

  • The problem occurs when trying to list the first offer:

  • It generates an address using id = 1

  • This generated address matches the address of the second offer (which was created with offerId = 1

  • The contract checks if this address already exists and also creates an offerAddress using the offerId which will correspond to the next offerAddress generated by the id while creating offer.

//PreMarkets.sol
//listOffer() function.
​
326: if (stockInfo.offer != address(0x0)) {
revert OfferAlreadyExist(); //will always revert
328: }
​
​
364: address offerAddr = GenerateAddress.generateOfferAddress(stockInfo.id);
if (offerInfoMap[offerAddr].authority != address(0x0)) {
revert OfferAlreadyExist(); // Will always revert
367: }
​
​
​

  • Since this address belongs to the second offer, the check fails, and the transaction reverts.

  • This mismatch between address generation in createOffer() and listOffer() causes all attempts to list offers after the first one to fail due to address collisions. The root cause is the inconsistency between the offerId used for address generation and the offerId stored with the offer, which will always cause a DoS.

Impact

  • The contract's address generation mismatch causes a Denial of Service (DoS) vulnerability where only the first offer can be listed, while all subsequent listing attempts fail due to the "OfferAlreadyExist" check, effectively rendering the marketplace unusable for multiple offers and severely limiting its functionality.

Tools Used

Manual Review

Recommendations

  • Use a separate mechanism to generate listing addresses.

// library Generate Address
+ function generateListOfferAddress(uint256 _id) internal pure returns (address) {
+ return address(uint160(uint256(keccak256(abi.encode(_id, "list")))));
+ }

Updates

Lead Judging Commences

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