Initialization functions can be front-run

[code423n4]

Handle

broccolirob

Vulnerability details

The Factory, Collateral, and Incentivizer contracts have initialize functions that can be front-run, allowing an attacker to initialize the contracts with incorrect values.

• Factory.sol L59-77
• Collateral.sol L35-41
• Incentivizer.sol L45-51

Impact

If the malicious intialization call is not detected immediately, the attacker can steal funds once the protocol is up and running.

Proof of Concept

Perennial deploys their Collateral contract. Eve front-runs its initialize function, and sets the factory address to a malicious contract of her own design. factory.treasury() and factory.treasury(product) now return Eve's address, allowing her to steal all fees from all products across the protocol.

Tools Used

Manual Analysis and Hardhat.

Recommended Mitigation Steps

Use OpenZeppelin's TransparentUpgradeableProxy contract to deploy these contracts. That way the initialize function and its arguments can be passed in as calldata to the Proxy's third constructor argument and run in the same transaction as the deployment.

[kbrizzle]

All deployable contracts in Perennial have been built to comply with the OZ upgrades plugin.

This deployment library handles cases such as preventing malicious front-running. Further, we will be validating that all params are correctly initialized as part of the deployment scripts.

Given that this information was not specified in the documentation, we're comfortable with calling this a 0 (Non-critical) issue.

[GalloDaSballo]

Agree with finding and severity

[GalloDaSballo]

See Poc from warden for mitigation

[eldadp]

Given that this information was not specified in the documentation I think this finding should be rewarded.