Lines of code

https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v1/FiatTokenV1.sol#L54 https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v2/FiatTokenV2.sol#L84

Vulnerability details

Impact

The state variable initialized will be set to True in the storage slot in the proxy contract after calling FiatTokenV1.initialize function. When the implementation is upgraded to FiatTokenV2, and calling initialise from proxy will revert as require fails on line https://github.com/code-423n4/2022-02-jpyc/blob/cfc018384dd1d71febaa57f0576cb51f5d9c7e07/contracts/v2/FiatTokenV2.sol#L84

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Here is a sample test snippet

  it('upgrade from V1 to V2 should fail after calling initialise', async function ()
  {
    const { receipt } = await this.token.upgradeToAndCall(fiatToken2.address, _data(masterMinter, pauser, blocklister, owner), { from: owner });
    expect(receipt.logs.filter(({ event }) => event === 'Upgraded').length).to.be.equal(1);
    expectEvent(receipt, 'Upgraded', { implementation: fiatToken2.address });
  });

One will get reverted with reason string FiatToken: contract is already initialized.

Tools Used

Recommended Mitigation Steps

Let FiatTokenV2 inherit FiatTokeV1. Then one just need to add extra variables and functions without having to worry about initialise and storage collision, and code repetition.

code-423n4 / 2022-02-jpyc-findings