Tadle

Summary

The PreMarkets contract has inconsistent behavior in how the offerId is incremented in the createOffer and createTaker functions. Specifically, the offerId is incremented before updating StockInfo in the createOffer function, but it is incremented after updating StockInfo in the createTaker function. This inconsistency could lead to confusion and potential data inconsistencies.

Vulnerability Details

• Location:

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

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

• Discrepancy:

  • In the createOffer function, offerId is incremented before updating the StockInfo.

  • In the createTaker function, offerId is incremented after updating the StockInfo.

• Code in createOffer:

  offerId = offerId + 1;

  ​

  /// @dev update stock info

  stockInfoMap[stockAddr] = StockInfo({

  id: offerId, // Using incremented offerId here

  // other attributes ...

  });

• Code in createTaker:

  offerInfo.usedPoints = offerInfo.usedPoints + _points;

  ​

  /// @dev update stock info

  stockInfoMap[stockAddr] = StockInfo({

  id: offerId, // Using non-incremented offerId here

  // other attributes ...

  });

  ​

  offerId = offerId + 1;

Impact

• Data Inconsistency: The inconsistency can lead to situations where stockInfo entries might have unexpected or duplicate id values, causing bugs and data inconsistency issues.

• Maintenance Difficulty: Inconsistent increment patterns can confuse developers and maintainers, increasing the likelihood of future bugs being introduced.

• Potential Vulnerabilities: If other functions or contracts depend on a specific pattern for offerId increments, the inconsistency could introduce unforeseen vulnerabilities or logical errors.

Tools Used

Manual code review

Recommendations

To resolve the inconsistency and ensure a standard approach, it is recommended to increment the offerId after updating the StockInfo in both functions. This change makes the code flow more logical, indicating that the current offerId is fully utilized before moving to the next ID.

Suggested Change for createOffer Function:

Increment the offerId after using it in StockInfo.

Updated createOffer function: