Tadle

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

offerId inconsistency

Summary

The current implementation of the createOffer function causes a mismatch between the offerId used to generate addresses and the offerId stored in the contract mappings.

Vulnerability Details

The current implementation of the createOffer function causes a mismatch between the offerId used to generate addresses and the offerId stored in the contract mappings. This discrepancy can lead to data integrity issues, where the addresses (makerAddr, offerAddr, stockAddr) do not correspond to the offerId used in the contract's state. Consequently, it can cause incorrect or unintended behavior when accessing or querying offer information, potentially leading to failures or security vulnerabilities.

Impact

Proof of Concept (PoC):
Function Flow:
The offerId is used to generate addresses for the maker, offer, and stock:
```javascript
address makerAddr = GenerateAddress.generateMakerAddress(offerId);
address offerAddr = GenerateAddress.generateOfferAddress(offerId);
address stockAddr = GenerateAddress.generateStockAddress(offerId);
```
The offerId is then incremented:
```javascript
offerId = offerId + 1;
```
The mappings `(makerInfoMap, offerInfoMap, stockInfoMap)` are updated with the incremented `offerId`, but the addresses were generated using the previous value of `offerId`.
Issue:
The addresses generated with the old `offerId` will not correspond to the `offerId` stored in the mappings since the offerId is incremented before storing the data.
Example:
Lets assume `offerId` is initially 1.
Addresses are generated using offerId which is 1
offerId is incremented offerId = offerId + 1 which is now 2 .
Mappings are updated with `offerId = 2`, but the addresses generated are for `offerId = 1`.
Verification:
Querying the mappings with `offerId = 2` will not return the correct addresses, as they were generated with `offerId = 1`.

Tools Used

Manual review

Recommendations

To ensure consistency between the generated `addresses` and `offerId`, modify the `createOffer` function to `increment offerId` before generating the addresses.
see below:

Increment offerId Before Generating Addresses:

Move the increment operation for offerId before the address generation:
```javascript
offerId = offerId + 1;

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

```
This ensures that the `offerId` used for generating addresses is the same as the `offerId` stored in the mappings.

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.