QA report

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L74

Vulnerability details

Impact

In FiaTokenV2.sol the initialize() function can only be called once and it sets the owner and other very important values in storage. Since it's not called in any deployment script this means that an attacker can monitor the blockchain byte code and automatically call the initialize() function setting themself as the owner before the protocol has the chance to do so.

Proof of Concept

https://github.com/code-423n4/2022-02-jpyc/blob/main/contracts/v2/FiatTokenV2.sol#L74

Tools Used

Manual code review

Recommended Mitigation Steps

The initialize() function in FiatTokenV2.sol should be called in a deployment script so an attacker wont be able to call it first.

[thurendous]

duplicate of #35

[thurendous]

For this issue, we are aware of this and have already decided to include initializing the contracts in the deploying scripts. So this should not be a problem. We will initialize both the implementation contract itself and the proxy contract storage. Like mentioned in#35 , including the initialization in the constructor can also be a an option for us.

For the reasons above, we argue this one is (1)low-risk.

[jack-the-pug]

It's a valid low or non-critical. I'll go with non for this one.

[CloudEllie]

Since this issue was downgraded to a QA level, and the warden did not submit a separate QA report, we've renamed this one to "QA report" for consistency.

The original title, for the record, was "FiatTokenV2.sol initialize() function can be called by an attacker first."