Explicitly test EVMScriptRunner.protectState modifier

Test setAppId also
aragon · Jul 13, 2019 · 0122188 · 0122188
1 parent c85d34e
commit 0122188
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 0 deletions.
diff --git a/contracts/test/mocks/evmscript/EVMScriptExecutorMalicious.sol b/contracts/test/mocks/evmscript/EVMScriptExecutorMalicious.sol
@@ -0,0 +1,24 @@
+pragma solidity ^0.4.24;
+
+import "../../../common/UnstructuredStorage.sol";
+import "../../../evmscript/executors/BaseEVMScriptExecutor.sol";
+import "../../../apps/AppStorage.sol";
+import "../../../kernel/IKernel.sol";
+
+
+contract EVMScriptExecutorMalicious is BaseEVMScriptExecutor, AppStorage {
+    bytes32 internal constant EXECUTOR_TYPE = keccak256("MALICIOUS_SCRIPT");
+
+    function execScript(bytes script, bytes, address[]) external isInitialized returns (bytes) {
+        // Use the script bytes variable to toggle between calling setKernel() and setAppId()
+        if (script[5] == 0x0) {
+            setKernel(IKernel(address(1)));
+        } else {
+            setAppId(bytes32(1));
+        }
+    }
+
+    function executorType() external pure returns (bytes32) {
+        return EXECUTOR_TYPE;
+    }
+}
diff --git a/test/contracts/evmscript/evm_script.js b/test/contracts/evmscript/evm_script.js
@@ -19,6 +19,7 @@ const ExecutionTarget = artifacts.require('ExecutionTarget')
 const EVMScriptExecutorMock = artifacts.require('EVMScriptExecutorMock')
 const EVMScriptExecutorNoReturnMock = artifacts.require('EVMScriptExecutorNoReturnMock')
 const EVMScriptExecutorRevertMock = artifacts.require('EVMScriptExecutorRevertMock')
+const EVMScriptExecutorMalicious = artifacts.require('EVMScriptExecutorMalicious')
 const EVMScriptRegistryConstantsMock = artifacts.require('EVMScriptRegistryConstantsMock')
 
 const ZERO_ADDR = '0x0000000000000000000000000000000000000000'
@@ -359,6 +360,39 @@ contract('EVM Script', ([_, boss]) => {
       })
     })
 
+    context('> State changing script', () => {
+      beforeEach(async () => {
+        // Deploy malicious executor
+        const scriptExecutorMalicious = await EVMScriptExecutorMalicious.new()
+
+        // Install mock reverting executor onto registry
+        await acl.createPermission(boss, evmScriptReg.address, REGISTRY_ADD_EXECUTOR_ROLE, boss, { from: boss })
+        const receipt = await evmScriptReg.addScriptExecutor(scriptExecutorMalicious.address, { from: boss })
+
+        // Sanity check it's at spec ID 1
+        const executorSpecId = getEventArgument(receipt, 'EnableExecutor', 'executorId')
+        const [, executorEnabled] = await evmScriptReg.executors(executorSpecId)
+        assert.equal(executorSpecId, 1, 'EVMScriptExecutorMalicious should be installed as spec ID 1')
+        assert.isTrue(executorEnabled, "malicious executor should be enabled")
+      })
+
+      it('fails if the kernel address changes', async () => {
+        // EVMScriptExecutorMalicious will call setKernel() 
+        // if the address is zero
+        const action = { to: ZERO_ADDR, calldata: "0x0" }
+        const script = encodeCallScript([action])
+        await assertRevert(scriptRunnerApp.runScript(script), reverts.EVMRUN_PROTECTED_STATE_MODIFIED)        
+      })
+
+      it('fails if appId changes', async () => {
+        // EVMScriptExecutorMalicious will call setAppId()
+        // if the address is NOT zero
+        const action = { to: "0x"+"ff".repeat(20), calldata: "0x0" }
+        const script = encodeCallScript([action])
+        await assertRevert(scriptRunnerApp.runScript(script), reverts.EVMRUN_PROTECTED_STATE_MODIFIED)        
+      })
+    })
+
     context('> CallsScript', () => {
       let callsScriptBase, executionTarget