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

`listOffer` Unsafely References Fungible Identifiers

auditweiler

Summary

The listOffer function depends upon identifiers which may have already been used, assuming them to be unique.

Vulnerability Details

The PreMarkets contract uses unsafe identifier logic resulting in collisions that lead to denial of service for core functionality.

Notice the following flow in createOffer:

/// @dev generate address for maker, offer, stock.
address makerAddr = GenerateAddress.generateMakerAddress(offerId); /// @audit uses_preincrement_offerId
address offerAddr = GenerateAddress.generateOfferAddress(offerId); /// @audit uses_preincrement_offerId
address stockAddr = GenerateAddress.generateStockAddress(offerId); /// @audit uses_preincrement_offerId
​
/// @custom:snip
​
offerId = offerId + 1; /// @audit increment_global_offerId
​
/// @custom:snip
​
/// @dev update offer info
offerInfoMap[offerAddr] = OfferInfo({
id: offerId, /// @audit uses_postincrement_offerId
​
/// @custom:snip
​
/// @dev update stock info
stockInfoMap[stockAddr] = StockInfo({
id: offerId, /// @audit uses_postincrement_offerId

In this sequence, it is clear to see that whenever createOffer is invoked, it is possible to inadvertently use identifiers that are already being used in previously-created elements of the offerInfoMap and stockInfoMap.

This is to say that different offers can inadvertently share the same state.

PreMarkets.t.sol

In the sequence below, we demonstrate how two calls to createOffer (i.e. through the nondeterminism of shared access to the blockchain) can result in denial of service:

/// @notice Multiple calls to `createOffer` can inadvertently
/// @notice mutate the context of another.
/// @notice For simplicity, we perform all actions as the single
/// @notice `user`, however this is not a requirement, as different
/// @notice users can DoS one-another due to the same flaw.
function test_ask_offer_turbo_eth_dos() public {
vm.startPrank(user);
​
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
​
/// @notice Toggle allow the transaction to either revert or succeed.
bool makeAnotherOffer = true;
​
/// @notice When `makeAnotherOffer` is true, we will have the
/// @notice same user create another offer.
if (makeAnotherOffer) {
preMarktes.createOffer{value: 0.012 * 1e18}(
CreateOfferParams(
marketPlace,
address(weth9),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
}
​
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker{value: 0.005175 * 1e18}(offerAddr, 500);
​
address stock1Addr = GenerateAddress.generateStockAddress(1);
​
/// @audit When `makeAnotherOffer` is true, it is impossible for
/// @audit the owner to create a listing. In this instance, the
/// @audit caller inadvertently returns their original offer
/// @audit to an uninitialized status, locking their capital.
if (makeAnotherOffer) vm.expectRevert("Mismatched Marketplace status");
preMarktes.listOffer(stock1Addr, 0.006 * 1e18, 12000);
​
}

Impact

Inability to listOffer through the action of indepenent actors, resulting in eventual inability to recouperate capital via the intended process of refunding a listing.

Tools Used

Manual Review

Recommendations

Refactor to safely isolate offers from the context of one-another.

Updates

Lead Judging Commences

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

Give us feedback!