diff --git a/.prettierignore b/.prettierignore index 92422bc..6d14728 100644 --- a/.prettierignore +++ b/.prettierignore @@ -5,6 +5,11 @@ artifacts build dist README.md -contracts/v2-vault-and-gas/libraries/EtherSend.sol coverage -coverage.json \ No newline at end of file +coverage.json + +diffs +contracts/v2-vault-and-gas +contracts/v3-relayer-registry +contracts/v4-patch/TornadoStakingRewards.sol +contracts/v4-patch/TornadoStakingRewards.sol \ No newline at end of file diff --git a/.prettierrc b/.prettierrc index f532b1b..11bd560 100644 --- a/.prettierrc +++ b/.prettierrc @@ -8,8 +8,7 @@ { "files": "*.sol", "options": { - "singleQuote": false, - "printWidth": 130 + "singleQuote": false } } ] diff --git a/config.js b/config.js index 65e461f..fcf03ba 100644 --- a/config.js +++ b/config.js @@ -2,4 +2,10 @@ module.exports = { governance: '0x5efda50f22d34F262c29268506C5Fa42cB56A1Ce', TORN: '0x77777FeDdddFfC19Ff86DB637967013e6C6A116C', tornWhale: '0xF977814e90dA44bFA03b6295A0616a897441aceC', + ens: '0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e', + feeManager: '0x5f6c97C6AD7bdd0AE7E0Dd4ca33A4ED3fDabD4D7', + staking: '0x2FC93484614a34f26F7970CBB94615bA109BB4bf', + registry: '0x58E8dCC13BE9780fC42E8723D8EaD4CF46943dF2', + compensations: '0xFA4C1f3f7D5dd7c12a9Adb82Cd7dDA542E3d59ef', + vault: '0x2F50508a8a3D323B91336FA3eA6ae50E55f32185', } diff --git a/contracts/v4-patch/GovernancePatchUpgrade.sol b/contracts/v4-patch/GovernancePatchUpgrade.sol index 02ec8c6..3410351 100644 --- a/contracts/v4-patch/GovernancePatchUpgrade.sol +++ b/contracts/v4-patch/GovernancePatchUpgrade.sol @@ -38,7 +38,10 @@ contract GovernancePatchUpgrade is GovernanceStakingUpgrade { proposalCodehash := extcodehash(target) } - require(proposalCodehash == proposalCodehashes[proposalId], "Governance::propose: metamorphic contracts not allowed"); + require( + proposalCodehash == proposalCodehashes[proposalId], + "Governance::propose: metamorphic contracts not allowed" + ); super.execute(proposalId); } diff --git a/contracts/v4-patch/PatchProposal.sol b/contracts/v4-patch/PatchProposal.sol index 2ca254d..158e6c4 100644 --- a/contracts/v4-patch/PatchProposal.sol +++ b/contracts/v4-patch/PatchProposal.sol @@ -3,10 +3,8 @@ pragma solidity ^0.6.12; pragma experimental ABIEncoderV2; -import { SafeMath } from "@openzeppelin/contracts/math/SafeMath.sol"; import { LoopbackProxy } from "../v1/LoopbackProxy.sol"; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; -import { Address } from "@openzeppelin/contracts/utils/Address.sol"; import { GovernancePatchUpgrade } from "./GovernancePatchUpgrade.sol"; import { TornadoStakingRewards } from "./TornadoStakingRewards.sol"; @@ -16,96 +14,56 @@ interface Proxy { function upgradeTo(address newImplementation) external; } -/** - * @notice Contract which should help the proposal deploy the necessary contracts. - */ -contract PatchProposalContractsFactory { - /** - * @notice Create a new TornadoStakingRewards contract. - * @param governance The address of Tornado Cash Goveranance. - * @param torn The torn token address. - * @param registry The address of the relayer registry. - * @return The address of the new staking contract. - */ - function createStakingRewards( - address governance, - address torn, - address registry - ) external returns (address) { - return address(new TornadoStakingRewards(governance, torn, registry)); - } - - /** - * @notice Create a new RelayerRegistry contract. - * @param torn The torn token address. - * @param governance The address of Tornado Cash Goveranance. - * @param ens The ens registrar address. - * @param staking The TornadoStakingRewards contract address. - * @return The address of the new registry contract. - */ - function createRegistryContract( - address torn, - address governance, - address ens, - address staking, - address feeManager - ) external returns (address) { - return address(new RelayerRegistry(torn, governance, ens, staking, feeManager)); - } -} - /** * @notice Proposal which should patch governance against the metamorphic contract replacement vulnerability. */ contract PatchProposal { - using SafeMath for uint256; - using Address for address; + // Address of the staking proxy + address public constant stakingProxyAddress = 0x2FC93484614a34f26F7970CBB94615bA109BB4bf; - address public constant feeManagerAddress = 0x5f6c97C6AD7bdd0AE7E0Dd4ca33A4ED3fDabD4D7; - address public constant ensAddress = 0x00000000000C2E074eC69A0dFb2997BA6C7d2e1e; - address public immutable registry = 0x58E8dCC13BE9780fC42E8723D8EaD4CF46943dF2; + // Address of the registry proxy + address public constant registryProxyAddress = 0x58E8dCC13BE9780fC42E8723D8EaD4CF46943dF2; + // Address of the gas compensation vault + address public constant gasCompensationVaultAddress = 0xFA4C1f3f7D5dd7c12a9Adb82Cd7dDA542E3d59ef; + + // Address of the user vault (not replaced) + address public constant userVaultAddress = 0x2F50508a8a3D323B91336FA3eA6ae50E55f32185; + + // Address of the governance proxy + address payable public constant governanceProxyAddress = 0x5efda50f22d34F262c29268506C5Fa42cB56A1Ce; + + // Torn token IERC20 public constant TORN = IERC20(0x77777FeDdddFfC19Ff86DB637967013e6C6A116C); - PatchProposalContractsFactory public immutable patchProposalContractsFactory; + // These are the contracts we deployed + address public immutable deployedStakingContractAddress; + address public immutable deployedRelayerRegistryImplementationAddress; - constructor(address _patchProposalContractsFactory) public { - patchProposalContractsFactory = PatchProposalContractsFactory(_patchProposalContractsFactory); + constructor(address _deployedStakingContractAddress, address _deployedRelayerRegistryImplementationAddress) + public + { + deployedStakingContractAddress = _deployedStakingContractAddress; + deployedRelayerRegistryImplementationAddress = _deployedRelayerRegistryImplementationAddress; } /// @notice Function to execute the proposal. function executeProposal() external { - // address(this) has to be governance - address payable governance = payable(address(this)); - - // Get the two contracts gov depends on - address gasComp = address(GovernancePatchUpgrade(governance).gasCompensationVault()); - address vault = address(GovernancePatchUpgrade(governance).userVault()); - // Get the old staking contract - TornadoStakingRewards oldStaking = TornadoStakingRewards(address(GovernancePatchUpgrade(governance).Staking())); + TornadoStakingRewards oldStaking = TornadoStakingRewards(stakingProxyAddress); // Get the small amount of TORN left oldStaking.withdrawTorn(TORN.balanceOf(address(oldStaking))); // And create a new staking contract - TornadoStakingRewards newStaking = TornadoStakingRewards( - patchProposalContractsFactory.createStakingRewards(address(governance), address(TORN), registry) - ); - - // And a new registry implementation - address newRegistryImplementationAddress = patchProposalContractsFactory.createRegistryContract( - address(TORN), - address(governance), - ensAddress, - address(newStaking), - feeManagerAddress - ); + TornadoStakingRewards newStaking = TornadoStakingRewards(deployedStakingContractAddress); // Upgrade the registry proxy - Proxy(registry).upgradeTo(newRegistryImplementationAddress); + Proxy(registryProxyAddress).upgradeTo(deployedRelayerRegistryImplementationAddress); // Now upgrade the governance to the latest stuff - LoopbackProxy(payable(governance)).upgradeTo(address(new GovernancePatchUpgrade(address(newStaking), gasComp, vault))); + LoopbackProxy(governanceProxyAddress).upgradeTo( + address(new GovernancePatchUpgrade(address(newStaking), gasCompensationVaultAddress, userVaultAddress)) + ); } } diff --git a/contracts/v4-patch/metamorphic/MetamorphicContractFactory.sol b/contracts/v4-patch/metamorphic/MetamorphicContractFactory.sol index 5aeb75c..409db3a 100644 --- a/contracts/v4-patch/metamorphic/MetamorphicContractFactory.sol +++ b/contracts/v4-patch/metamorphic/MetamorphicContractFactory.sol @@ -127,16 +127,22 @@ contract MetamorphicContractFactory { */ constructor(bytes memory transientContractInitializationCode) public { // assign the initialization code for the metamorphic contract. - _metamorphicContractInitializationCode = (hex"5860208158601c335a63aaf10f428752fa158151803b80938091923cf3"); + _metamorphicContractInitializationCode = ( + hex"5860208158601c335a63aaf10f428752fa158151803b80938091923cf3" + ); // calculate and assign keccak256 hash of metamorphic initialization code. - _metamorphicContractInitializationCodeHash = keccak256(abi.encodePacked(_metamorphicContractInitializationCode)); + _metamorphicContractInitializationCodeHash = keccak256( + abi.encodePacked(_metamorphicContractInitializationCode) + ); // store the initialization code for the transient contract. _transientContractInitializationCode = transientContractInitializationCode; // calculate and assign keccak256 hash of transient initialization code. - _transientContractInitializationCodeHash = keccak256(abi.encodePacked(_transientContractInitializationCode)); + _transientContractInitializationCodeHash = keccak256( + abi.encodePacked(_transientContractInitializationCode) + ); } /* solhint-disable function-max-lines */ @@ -214,7 +220,10 @@ contract MetamorphicContractFactory { } /* solhint-enable no-inline-assembly */ // ensure that the contracts were successfully deployed. - require(deployedMetamorphicContract == metamorphicContractAddress, "Failed to deploy the new metamorphic contract."); + require( + deployedMetamorphicContract == metamorphicContractAddress, + "Failed to deploy the new metamorphic contract." + ); // initialize the new metamorphic contract if any data or value is provided. if (data.length > 0 || msg.value > 0) { @@ -281,7 +290,10 @@ contract MetamorphicContractFactory { } /* solhint-enable no-inline-assembly */ // ensure that the contracts were successfully deployed. - require(deployedMetamorphicContract == metamorphicContractAddress, "Failed to deploy the new metamorphic contract."); + require( + deployedMetamorphicContract == metamorphicContractAddress, + "Failed to deploy the new metamorphic contract." + ); // initialize the new metamorphic contract if any data or value is provided. if (data.length > 0 || msg.value > 0) { @@ -412,7 +424,11 @@ contract MetamorphicContractFactory { * @param salt bytes32 The nonce passed into CREATE2 by metamorphic contract. * @return Address of the corresponding metamorphic contract. */ - function findMetamorphicContractAddress(bytes32 salt) external view returns (address metamorphicContractAddress) { + function findMetamorphicContractAddress(bytes32 salt) + external + view + returns (address metamorphicContractAddress) + { // determine the address where the metamorphic contract will be deployed. metamorphicContractAddress = _getMetamorphicContractAddress(salt); } @@ -424,7 +440,11 @@ contract MetamorphicContractFactory { * transient contract. * @return Address of the corresponding transient contract. */ - function findTransientContractAddress(bytes32 salt) external view returns (address transientContractAddress) { + function findTransientContractAddress(bytes32 salt) + external + view + returns (address transientContractAddress) + { // determine the address where the transient contract will be deployed. transientContractAddress = _getTransientContractAddress(salt); } @@ -442,14 +462,20 @@ contract MetamorphicContractFactory { returns (address metamorphicContractAddress) { // determine the address of the metamorphic contract. - metamorphicContractAddress = _getMetamorphicContractAddressWithConstructor(_getTransientContractAddress(salt)); + metamorphicContractAddress = _getMetamorphicContractAddressWithConstructor( + _getTransientContractAddress(salt) + ); } /** * @dev View function for retrieving the initialization code of metamorphic * contracts for purposes of verification. */ - function getMetamorphicContractInitializationCode() external view returns (bytes memory metamorphicContractInitializationCode) { + function getMetamorphicContractInitializationCode() + external + view + returns (bytes memory metamorphicContractInitializationCode) + { return _metamorphicContractInitializationCode; } @@ -469,7 +495,11 @@ contract MetamorphicContractFactory { * @dev View function for retrieving the initialization code of transient * contracts for purposes of verification. */ - function getTransientContractInitializationCode() external view returns (bytes memory transientContractInitializationCode) { + function getTransientContractInitializationCode() + external + view + returns (bytes memory transientContractInitializationCode) + { return _transientContractInitializationCode; } @@ -477,7 +507,11 @@ contract MetamorphicContractFactory { * @dev View function for retrieving the keccak256 hash of the initialization * code of transient contracts for purposes of verification. */ - function getTransientContractInitializationCodeHash() external view returns (bytes32 transientContractInitializationCodeHash) { + function getTransientContractInitializationCodeHash() + external + view + returns (bytes32 transientContractInitializationCodeHash) + { return _transientContractInitializationCodeHash; } @@ -532,7 +566,11 @@ contract MetamorphicContractFactory { * that has been deployed via a transient contract given the address of the * transient contract. */ - function _getMetamorphicContractAddressWithConstructor(address transientContractAddress) internal pure returns (address) { + function _getMetamorphicContractAddressWithConstructor(address transientContractAddress) + internal + pure + returns (address) + { // determine the address of the metamorphic contract. return address( @@ -560,7 +598,10 @@ contract MetamorphicContractFactory { modifier containsCaller(bytes32 salt) { // prevent contract submissions from being stolen from tx.pool by requiring // that the first 20 bytes of the submitted salt match msg.sender. - require(address(bytes20(salt)) == msg.sender, "Invalid salt - first 20 bytes of the salt must match calling address."); + require( + address(bytes20(salt)) == msg.sender, + "Invalid salt - first 20 bytes of the salt must match calling address." + ); _; } } diff --git a/hardhat.config.js b/hardhat.config.js index 1de94bc..52954c0 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -16,25 +16,36 @@ const config = require('./config') module.exports = { solidity: { compilers: [ - { - version: '0.6.12', - settings: { - optimizer: { - enabled: true, - runs: 1000, - }, - }, - }, { version: '0.5.6', settings: { optimizer: { enabled: true, - runs: 1000, + runs: 1_000, + }, + }, + }, + { + version: '0.6.12', + settings: { + optimizer: { + enabled: true, + runs: 1_000_000, }, }, }, ], + overrides: { + 'contracts/v4-patch/PatchProposal.sol': { + version: '0.6.12', + settings: { + optimizer: { + enabled: true, + runs: 1, + }, + }, + }, + }, }, networks: { hardhat: { diff --git a/package.json b/package.json index d1fa8e2..2d60182 100644 --- a/package.json +++ b/package.json @@ -17,6 +17,7 @@ "test:f": "yarn prettier:fix && yarn test", "clean": "yarn prettier:fix && yarn lint", "compile": "yarn prettier:fix && yarn hardhat compile", + "_compile": "yarn hardhat compile", "coverage": "yarn hardhat coverage", "layout": "yarn hardhat check" }, diff --git a/test/patch/patch.test.js b/test/patch/patch.test.js index f2db310..605904a 100644 --- a/test/patch/patch.test.js +++ b/test/patch/patch.test.js @@ -54,11 +54,16 @@ describe('Gov Exploit Patch Upgrade Tests', () => { let proposalContractsDeployer let proposalDeployer + let registryDeployer + let stakingDeployer + let proposerBalanceInitial let torn let governance let metamorphicFactory + let registryImplementation + let staking let exploit = { hacker: undefined, @@ -134,7 +139,10 @@ describe('Gov Exploit Patch Upgrade Tests', () => { initialProposalDeployer = await ethers.getContractFactory('InitialProposal') maliciousProposalDeployer = await ethers.getContractFactory('MaliciousProposal') - proposalContractsDeployer = await ethers.getContractFactory('PatchProposalContractsFactory') + + stakingDeployer = await ethers.getContractFactory('TornadoStakingRewards') + registryDeployer = await ethers.getContractFactory('RelayerRegistry') + proposalDeployer = await ethers.getContractFactory('PatchProposal') // Metamorphic & Exploit @@ -146,6 +154,16 @@ describe('Gov Exploit Patch Upgrade Tests', () => { initialProposalImpl = await initialProposalDeployer.deploy() maliciousProposalImpl = await maliciousProposalDeployer.deploy() + staking = await stakingDeployer.deploy(config.governance, config.TORN, config.registry) + + registryImplementation = await registryDeployer.deploy( + config.TORN, + config.governance, + config.ens, + config.staking, + config.feeManager, + ) + exploit.address = await metamorphicFactory.findMetamorphicContractAddress(exploit.salt) // Snapshot @@ -159,16 +177,16 @@ describe('Gov Exploit Patch Upgrade Tests', () => { snapshotId = await takeSnapshot() }) - it('Should be able to execute the proposal', async () => { + it.only('Should be able to execute the proposal', async () => { // Load these storage variables for comparison const oldVaultAddr = await governance.userVault() const oldGasCompAddr = await governance.gasCompensationVault() const oldStaking = await governance.Staking() - // Start proposing + // Deploy the proposal - const proposal = await proposalDeployer.deploy((await proposalContractsDeployer.deploy()).address) + const proposal = await proposalDeployer.deploy(staking.address, registryImplementation.address) // Propose @@ -194,7 +212,9 @@ describe('Gov Exploit Patch Upgrade Tests', () => { await ethers.provider.send('evm_mine', []) - await governance.execute(proposalId) + const response = await governance.execute(proposalId) + + console.log(await response.wait()) const newVaultAddr = await governance.userVault() const newGasCompAddr = await governance.gasCompensationVault()