Inconsistent upgradability Pattern
Summary
PreMarkets, DeliveryPlace and TokenManager have their respective storage contract, means to be upgradeable. However, some parent contract inherited by them are not upgradeable and has missing storage gaps.
Vulnerability Details
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L27
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/storage/PerMarketsStorage.sol#L36
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/utils/Rescuable.sol#L12
As can be seen above, gap storage has been implemented by PerMarketsStorage contract inherited by the 1PreMarkets. However, no gaps storage in implemented on Rescuable, OwnableandPausable` contract.
For Openzeppelin contracts, it is recommended the use of upgradeable contract such as OwnableUpgradeable and PausableUpgradeable.
Impact
Storage slot might get corrupted on upgrade and future upgrades might not be possible due to non-upgradeable contract.
Tools Used
Manual
Recommendations
Use OpenZeppelin/openzeppelin-contracts-upgradeable and add a storage gap to Rescuable.
Lead Judging Commences
finding-PreMarkets-abortBidTaker-amount-wrong-StockInfo-points
Valid high severity, due to incorrect computation of `depositAmount` within `abortBidTaker`, when aborting bid offers created by takers, the collateral refund will be completely wrong for the taker, and depending on the difference between the value of `points` and `amount`, it can possibly even round down to zero, causing definite loss of funds. If not, if points were worth less than the collateral, this could instead be used to drain the CapitalPool contract instead.
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.