code-423n4 / 2024-01-renft-findings

2 stars 0 forks source link

The `getCreate2Address` returns an incorrect address on the `zkSync Era chain`. #340

Closed c4-bot-5 closed 9 months ago

c4-bot-5 commented 9 months ago

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/Create2Deployer.sol#L84-L95 https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/Create2Deployer.sol#L67

Vulnerability details

Impact

The getCreate2Address returns an incorrect address on the zkSync Era chain.

Proof of Concept

The project claims to deploy the protocol on chains that support Gnosis Safe, the zkSync Era chain is one of them and since it's gaining more popularity every day I think it's a viable option for future deployment.

According to the official documentation from zkSync Era, the address derivation is different and the current way of calculating addresses won't work. The calculation process on era uses a different prefix. All deployments will fail.

export function create2Address(sender: Address, bytecodeHash: BytesLike, salt: BytesLike, input: BytesLike) {
  const prefix = ethers.utils.keccak256(ethers.utils.toUtf8Bytes("zksyncCreate2"));
  const inputHash = ethers.utils.keccak256(input);
  const addressBytes = ethers.utils.keccak256(ethers.utils.concat([prefix, ethers.utils.zeroPad(sender, 32), salt, bytecodeHash, inputHash])).slice(26);
  return ethers.utils.getAddress(addressBytes);
}

https://docs.zksync.io/build/developer-reference/differences-with-ethereum.html#create-create2

Tools Used

Manual Review

Recommended Mitigation Steps

Change how contract addresses are calculated on the zkSync Era chain.

Assessed type

Other

c4-pre-sort commented 9 months ago

141345 marked the issue as sufficient quality report

141345 commented 9 months ago

zksync chain support, more like feature improvement QA seems more appropriate

c4-pre-sort commented 9 months ago

141345 marked the issue as duplicate of #371

c4-judge commented 9 months ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 9 months ago

0xean marked the issue as grade-c