Lines of code
<https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/NounsDAOProxyV2.sol#L43&gt;
<https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/NounsDAOProxyV3.sol#L43&gt;

Vulnerability details

Impact

NounsDAOProxy contract may lose tracking its implementation address

Proof of Concept

One of the main vulnerabilities of upgradeable contracts is storing the implementation address in the beginning slots. This address is later used by proxy for delegatecall to the implementation contract. Although the Compounds GovernorBravoDelegator contract stores the implementation address in its first three slots, the equivalent slot in the proxy may be changed silently in the future. The NounsDAOProxy contracts (V1-V3) are similar to GovernorBravoDelegator contract which has this potential risk. To reduce this risk, storing the implementation address in an arbitrary, far from beginning slots is recommended. ERC1967 is intended to mitigate this risk by storing the address in implementation slot which is defined as:

    bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1)

By storing the implementation address in the implementation slot of the proxy contract, the issue of storage collision disappears.

Tools Used

Manual Review

Recommended Mitigation Steps

Pretend to use OpenZeppelin’s ERC1967 proxy

Assessed type

Upgradable

The text was updated successfully, but these errors were encountered:

All reactions