Tadle

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

Wrong ID stored against generated offerAddr, stockAddr can lead to change in authority of offerInfo

Summary

In createOffer function of PreMarkets.sol, the offerId used to generate virtual addresses of makerAddr, offerAddr, stockAddr is one less than what is stored against them in the mappings of offerInfoMap[offerAddr], stockInfoMap[stockAddr].

Reference:

offerId updated: https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L83
updated value of offerId used: https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L116-L146

Vulnerability Details

With the current implementation it is not possible to change authority of an offer but if in future upgrades, the following check in listOffer is removed, it could result in getting wrong stockInfo.stockId which can overwrite the data for offerInfo created in createOffer.

Impact

Users who create offers using createOffer can loose authority of their offers, thereby resulting in loss of funds used as collateral against those offers.

Tools Used

Manual Review

Recommendations

Update offerId after the creation of stockInfo and offerInfo in createOffer.

Updates

Lead Judging Commences

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