code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-02-jpyc/blob/main/test/v1/FiatTokenV1.test.js#L57-L67

Vulnerability details

Impact

Depending on the actual deployment code, FiatTokenV1.sol may be a potential target for the Denial Of Service (DOS) type of attack.

Proof of Concept

FiatTokenV1.sol inherits UUPSUpgradeable.sol. It is important that the contract is deployed and initialized in the same transaction to avoid any malicious frontrunning. FiatToken.sol may potentially have initialized variable be false, and if this happens, the malicious attacker would take over the control.

There are no codes for the deployment included in this contest, but the implementation of FiatTokenV1.test.js#L57-L67 seems to be susceptible.

  this.token = await FiatTokenV1.new() 
                                  // <---- the potential frontrunning attack could happen after the instance of this.token is created before initialize function is called
  await this.token.initialize(
    name,
    symbol,
    currency,
    decimals,
    masterMinter,
    pauser,
    blocklister,
    owner
  )

When deploying FiatTokenV1.sol's contract, the malicious attacker could monitor the Ethereum blockchain for bytecode that matches the contract and frontrun the initialize() transaction to gain ownership of the contract.

Here are related reports at the past code4arena competition (reference1 reference2).

Tools Used

Static analysis

Recommended Mitigation steps

Removing the usage of non-upgradeable compatible library (such as Ownable.sol), using the upgradeable compatible library (OwnableUpgradeable.sol), using Initializable.sol and following recommended pattern of upgradeable implementation by OpenZeppelin seems to be a good direction. However, existing codes use Ownable.sol at some contracts which adds some efforts to do the conversion. Another potential workaround can be to ensure that FiatToken.sol will be deployed and initialized in the same transaction or add logic to make initialize() function at FiatToken.sol to be callable only by the deployer.

thurendous commented 2 years ago

duplicate of #35

CloudEllie commented 2 years ago

Grouping this with warden's QA report, #52

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