Skip to content

Latest commit

 

History

History
24 lines (15 loc) · 1.41 KB

M-01.md

File metadata and controls

24 lines (15 loc) · 1.41 KB

PartyGovernanceNFT implementation constructor is allowed to receive ETH

Impact

The changeset included in the scope of the contest includes a particular modification in which the constructor of the PartyGovernanceNFT contract is now marked with the payable modifier.

https://github.com/code-423n4/2023-05-party/blob/main/contracts/party/PartyGovernanceNFT.sol#L77

-    constructor(IGlobals globals) PartyGovernance(globals) ERC721("", "") {
+    constructor(IGlobals globals) payable PartyGovernance(globals) ERC721("", "") {
         _GLOBALS = globals;
     }

This change would allow the contract to receive ETH while being constructed. However, the PartyGovernanceNFT is intended to be used behind proxies. A single reference implementation of the contract is deployed, and parties are constructed by creating proxies that reference this implementation.

This means that ETH sent while constructing the PartyGovernanceNFT contract will actually be sent to the implementation contract, and not to the real party instances, as these are deployed using proxies.

The reason for the change remains unclear. If this is not an accident and is a decision to save gas by avoiding the check, it is important to note that the savings would be extremely marginal over the risk of potentially sending ETH to the implementation contract.

Recommendation

Remove the payable modifier from the PartyGovernanceNFT constructor.