diff --git a/packages/package-utils/RetryProvider.js b/packages/package-utils/RetryProvider.js index 71ebd2bd7e..841a348112 100644 --- a/packages/package-utils/RetryProvider.js +++ b/packages/package-utils/RetryProvider.js @@ -8,10 +8,18 @@ class RetryProvider extends ethers.providers.StaticJsonRpcProvider { } static attemptCheck(err, attemptNumber) { - if (err.code === "CALL_EXCEPTION" || err.code === "UNPREDICTABLE_GAS_LIMIT") { + const allowedErrorCodes = ["CALL_EXCEPTION", "UNPREDICTABLE_GAS_LIMIT"]; + if (allowedErrorCodes.includes(err.code)) { console.log(`Got a ${err.code}, no retrying`); return false; } + + // I _think_ this means we're using solidity-coverage vs stock hardhat, but haven't dug in to it + if (err.error && err.error.data && err.error.data.length > 10 && err.error.data.substring(0, 10) === "0x08c379a0") { + console.log("Got a revert with reason, no retrying"); + return false; + } + console.log("Retrying RPC request #", attemptNumber); if (attemptNumber === 5) { return false; diff --git a/scripts/deployOldUpgradeableVersion.js b/scripts/deployOldUpgradeableVersion.js index f805b240b5..1bd45cc51b 100644 --- a/scripts/deployOldUpgradeableVersion.js +++ b/scripts/deployOldUpgradeableVersion.js @@ -8,9 +8,15 @@ const contract = require("@truffle/contract"); const { web3GetCode, setStorageSlot } = require("../helpers/test-helper"); const { ROOT_ROLE, RECOVERY_ROLE, ADMINISTRATION_ROLE, ARCHITECTURE_ROLE, ADDRESS_ZERO } = require("../helpers/constants"); -const colonyDeployed = {}; -const colonyNetworkDeployed = {}; -const deployedResolverAddresses = {}; +let colonyDeployed = {}; +let colonyNetworkDeployed = {}; +let deployedResolverAddresses = {}; + +module.exports.resetAlreadyDeployedVersionTracking = async () => { + colonyDeployed = {}; + colonyNetworkDeployed = {}; + deployedResolverAddresses = {}; +}; module.exports.deployOldExtensionVersion = async (contractName, interfaceName, implementationNames, versionTag, colonyNetwork) => { if (versionTag.indexOf(" ") !== -1) { @@ -116,14 +122,12 @@ module.exports.deployOldColonyVersion = async (contractName, interfaceName, impl colonyDeployed[interfaceName] = {}; } if (colonyDeployed[interfaceName][versionTag]) { - // Already deployed... if truffle's not snapshotted it away. See if there's any code there. + // Already deployed... note that if a snapshot revert is made without calling `resetAlreadyDeployedVersionTracking`, + // then this will break. const { resolverAddress } = colonyDeployed[interfaceName][versionTag]; - const code = await web3GetCode(resolverAddress); - if (code !== "0x") { - // Must also check it's registered - await module.exports.registerOldColonyVersion(resolverAddress, colonyNetwork); - return colonyDeployed[interfaceName][versionTag]; - } + // Must also check it's registered + await module.exports.registerOldColonyVersion(resolverAddress, colonyNetwork); + return colonyDeployed[interfaceName][versionTag]; } try { diff --git a/test/contracts-network/colony.js b/test/contracts-network/colony.js index 199b31e648..176f6bd4e5 100755 --- a/test/contracts-network/colony.js +++ b/test/contracts-network/colony.js @@ -413,6 +413,30 @@ contract("Colony", (accounts) => { await checkErrorRevert(colony.executeMetaTransaction(USER1, txData, r, s, v, { from: USER1 }), "metatransaction-signer-signature-mismatch"); }); + + it("not vulnerable to metatransactions / multicall vulnerability", async () => { + // https://blog.solidityscan.com/unveiling-the-erc-2771context-and-multicall-vulnerability-f96ffa5b499f + // Create an expenditure as a user + await colony.makeExpenditure(1, UINT256_MAX, 1); + + // Should not be able to multicall and cancel it as another user, pretending to be the first user + const expenditureId = await colony.getExpenditureCount(); + let txData1 = await colony.contract.methods.cancelExpenditure(expenditureId).encodeABI(); + + const METATRANSACTION_FLAG = ethers.utils.id("METATRANSACTION"); + + txData1 += METATRANSACTION_FLAG.slice(2) + USER0.slice(2); + + const txData2 = await colony.contract.methods.multicall([txData1]).encodeABI(); + + const { r, s, v } = await getMetaTransactionParameters(txData2, USER1, colony.address); + + // User 1 can't cancel the expenditure directly + await checkErrorRevert(colony.cancelExpenditure(expenditureId, { from: USER1 }), "colony-expenditure-not-owner"); + + // And can't via metatransaction using specially constructed malicious txdata + await checkErrorRevert(colony.executeMetaTransaction(USER1, txData2, r, s, v, { from: USER1 }), "colony-metatx-function-call-unsuccessful"); + }); }); describe("when executing a multicall transaction", () => { diff --git a/test/truffle-fixture.js b/test/truffle-fixture.js index 8a3373de1a..f5a60df48d 100644 --- a/test/truffle-fixture.js +++ b/test/truffle-fixture.js @@ -66,6 +66,7 @@ const ethers = require("ethers"); const { soliditySha3 } = require("web3-utils"); const truffleContract = require("@truffle/contract"); const createXABI = require("../lib/createx/artifacts/src/ICreateX.sol/ICreateX.json"); +const { resetAlreadyDeployedVersionTracking } = require("../scripts/deployOldUpgradeableVersion"); let postFixtureSnapshotId; @@ -83,6 +84,7 @@ const { getChainId, hardhatRevert, hardhatSnapshot, deployCreateXIfNeeded, isXda module.exports = async () => { if (postFixtureSnapshotId) { await hardhatRevert(hre.network.provider, postFixtureSnapshotId); + await resetAlreadyDeployedVersionTracking(); postFixtureSnapshotId = await hardhatSnapshot(hre.network.provider); return; }