Lines of code
<https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/Treasury.sol#L25-L34&gt;
<https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/Auction.sol#L31-L42&gt;
<https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/Token.sol#L23-L32&gt;
<https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/metadata/MetadataRenderer.sol#L25-L34&gt;

Vulnerability details

Impact

The deployment flow is based on the concept that the ZORA team can deploy implementation contracts and users will be able to deploy proxies. However, because manager is immutably set in the implementation contracts, this entire system will not work.

Proof of Concept

In each of Token, MetadataRenderer, Auction, Treasury, and Governor, the Manager address is immutably set in the constructor.

Constructors are called when the implementation contract is deployed, and the immutably set value will be hardcoded into the runtime bytecode.

As users deploy their proxies, they will not be able to overwrite this value, and will therefore not have the permissions needed to run their protocol.

Tools Used

Manual Review

Recommended Mitigation Steps

Change the deployment flow so that Manager is set in the initialize() contract, so that users deploying proxies are able to set their own Manager.

This seems like it slightly reduces security, since there will not be able to be a check that only the Manager can make the initialize() call. However, since the proxies are deployed within the same function call that they are initialized (within Manager:deploy()) there is no risk here.

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

All reactions