Beanstalk: The Finale

• Likelihood: High

• Impact: High

Summary

The contract address used in L2ContractMigrationFacet for generating EIP712 Domain Separators relies on the contract having the same address on both the L1 and L2, which isn't the case. If and when they differ, this will result in an inability to verify ownership of funds to transfer, meaning these transfers cannot occur.

Vulnerability Details

The signing of the EIP712 hash is performed on the L1, as per the documentation inside the codebase linked above and also evidenced by the fact that the chainId used in the domain separator is from C.getLegacyChainId(). However the verification of this signature for the moving of funds to the L2 balances relies on the contract address of the L2ContractMigrationFacet on the L2, not the contract against which the original hash that was signed was created. This will result in an incorrect hash being used to verify the signature and will result in a revert in the verification process if the contract addresses are different.

Contract addresses are computed using the keccak256 hash of the deployer's address (sender) and its nonce (the number of transactions sent by the sender). So even if the exact same contract code is used, the resulting contract address on different chains will be different unless it is deployed by the same account with the same nonce on each chain, which is highly unlikely in different networks.

I could not find any indication that there is deployment code which could leverage the CREATE2 opcode which would allow for deterministic contract deployments which would allow the use of ensuring a consistent address for the same contract deployed on both L1 and L2.

Impact

Any attempt by owners to redeem balances on the L2 will fail if the contract address is not the same.

Tools Used

Manual Review

Recommendations

Ensure that the L2ContractMigrationFacet._domainSeparatorV4 function uses the same domain separator as the creating hash. The chainId is already pegged to the legacy chain by storing the old and new chainids in C.sol - if the original contract address is not explicitly stored anywhere, then it needs to be included directly in this contract, or by a new migration specific constants file which can be referenced.

If desired, a factory contract could be deployed which ensures that the contracts on the L1 and L2 are the same. An example is linked above in the deterministic proxy pattern and there is information how to implement this on the openzeppelin site : https://blog.openzeppelin.com/evm-deterministic-deployments-made-easy-with-openzeppelin-defender

To avoid the dependency on identical contract addresses, we can simply store the previous address and in the example below we have a Migration contract which has the right legacy contract address in that was used in the L1 signing.

Given this is a migration only contract, using a hardcoded contract address for the legacy chain seems a fair choice here.