High Neon Llama
High
The old MarketFactory
is meant to be upgraded to a new implementation. The problem is that a new extensions
mapping was added between currently occupied storage slots. After the upgrade, the newly upgraded smart contract would be reading from storage slots that contain data no longer corresponding to the new storage layout. This would cause the system to break in an unpredictable manner, depending on the number of storage slots added as part of the upgrade.
As seen in the old storage layout:
IFactory public immutable oracleFactory;
ProtocolParameterStorage private _parameter;
mapping(address => mapping(address => bool)) public operators;
mapping(IOracleProvider => mapping(address => IMarket)) private _markets;
mapping(address => UFixed6) public referralFee;
And the new storage layout:
IFactory public immutable oracleFactory;
IVerifier public immutable verifier;
ProtocolParameterStorage private _parameter;
@> mapping(address => bool) public extensions;
mapping(address => mapping(address => bool)) public operators;
mapping(IOracleProvider => mapping(address => IMarket)) private _markets;
mapping(address => UFixed6) private _referralFees;
mapping(address => mapping(address => bool)) public signers;
The storage will be corrupted after the upgrade because the new extensions
mapping was introduced between already populated slots. The extensions
, operators
, _markets
, and _referralFees
will read and write to incorrect slots.
- Corrupted storage of the
MarketFactory
contract. - System would break in an unpredictable manner.
https://github.com/sherlock-audit/2024-08-perennial-v2-update-3/blob/main/perennial-v2/packages/perennial/contracts/MarketFactory.sol#L11-L35
https://github.com/equilibria-xyz/perennial-v2/blob/main/packages/perennial/contracts/MarketFactory.sol#L10-L25
https://arbiscan.io/address/0x046d6038811c6c14e81d5de5b107d4b7ee9b4cde#code#F1#L1
Manual Review
Place the extensions
mapping after the _referralFees
mapping so that both extensions
and signers
are added after all the occupied slots, avoiding storage corruption.