code-423n4 / 2021-04-vader-findings

1 stars 0 forks source link

Initialization can be front-run in USDV.sol #132

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

Given the public access, this is susceptible to front-running by an attacker who can initialize this with arbitrary assets before the deployer. Reinitialization will require contract redeployment because initialization can be done only once.

Reference: See finding #12 from Trail of Bits audit of Hermez Network: https://github.com/trailofbits/publications/blob/master/reviews/hermez.pdf

Proof of Concept

https://github.com/code-423n4/2021-04-vader/blob/3041f20c920821b89d01f652867d5207d18c8703/vader-protocol/contracts/USDV.sol#L54-L61

Tools Used

Manual Analysis

Recommended Mitigation Steps

Use a factory pattern that will deploy and initialize atomically to prevent front-running of the initialization, or

Ensure the deployment scripts are robust in case of a front-running attack or

Given that contracts are not using delegatecall proxy pattern, it is not required to use a separate init() function to initialize parameters when the same can be done in a constructor. If the reason for doing so is to get the deployment addresses of the various contracts which may not all be available at the same time then consider rearchitecting to create a “globals” contract which can hold all the globally required addresses of various contracts. (see Maple protocol’s https://github.com/maple-labs/maple-core/blob/develop/contracts/MapleGlobals.sol for example)

strictly-scarce commented 3 years ago

https://github.com/code-423n4/2021-04-vader-findings/issues/39

dmvt commented 3 years ago

duplicate of #18