Merge pull request #142 from gnosis/force_safe_setup

Don't allow transactions before setting up the Safe

Author: rmeissner

contracts/GnosisSafe.sol

@@ -181,16 +181,20 @@ contract GnosisSafe
     function checkSignatures(bytes32 dataHash, bytes memory data, bytes memory signatures, bool consumeHash)
         internal
     {
+        // Load threshold to avoid multiple storage loads
+        uint256 _threshold = threshold;
+        // Check that a threshold is set
+        require(_threshold > 0, "Threshold needs to be defined!");
         // Check that the provided signature data is not too short
-        require(signatures.length >= threshold.mul(65), "Signatures data too short");
+        require(signatures.length >= _threshold.mul(65), "Signatures data too short");
         // There cannot be an owner with address 0.
         address lastOwner = address(0);
         address currentOwner;
         uint8 v;
         bytes32 r;
         bytes32 s;
         uint256 i;
-        for (i = 0; i < threshold; i++) {
+        for (i = 0; i < _threshold; i++) {
             (v, r, s) = signatureSplit(signatures, i);
             // If v is 0 then it is a contract signature
             if (v == 0) {
@@ -200,7 +204,7 @@ contract GnosisSafe
                 // Check that signature data pointer (s) is not pointing inside the static part of the signatures bytes
                 // This check is not completely accurate, since it is possible that more signatures than the threshold are send.
                 // Here we only check that the pointer is not pointing inside the part that is being processed
-                require(uint256(s) >= threshold.mul(65), "Invalid contract signature location: inside static part");
+                require(uint256(s) >= _threshold.mul(65), "Invalid contract signature location: inside static part");
 
                 // Check that signature data pointer (s) is in bounds (points to the length of data -> 32 bytes)
                 require(uint256(s).add(32) <= signatures.length, "Invalid contract signature location: length not present");

test/gnosisSafeForceSafeSetup.js

@@ -0,0 +1,45 @@
+const utils = require('./utils/general')
+
+const GnosisSafe = artifacts.require("./GnosisSafe.sol")
+
+contract('GnosisSafe setup', function(accounts) {
+
+    let gnosisSafe
+    let executor = accounts[8]
+
+    const CALL = 0
+
+    it.only('should not be able to call execTransaction before setup', async () => {
+
+        // Create lightwallet
+        gnosisSafe = await utils.deployContract("deploying Gnosis Safe", GnosisSafe)
+
+        // Fund Safe
+        await web3.eth.sendTransaction({from: accounts[0], to: gnosisSafe.address, value: web3.toWei(1, 'ether')})
+        assert.equal(await web3.eth.getBalance(gnosisSafe.address).toNumber(), web3.toWei(1, 'ether'))
+
+        let sigs = "0x000000000000000000000000" + executor.replace('0x', '') + "0000000000000000000000000000000000000000000000000000000000000000" + "01"
+
+        await utils.assertRejects(
+            gnosisSafe.execTransaction(
+                accounts[0], web3.toWei(1, 'ether'), "0x", CALL, 0, 0, 0, 0, 0, sigs, {from: executor}
+            ),
+            "Should not be able to execute transaction before setup"
+        )
+
+        assert.equal(await web3.eth.getBalance(gnosisSafe.address).toNumber(), web3.toWei(1, 'ether'))
+
+        let setup = await gnosisSafe.setup([executor], 1, 0, "0x", 0, 0, 0, 0)
+        utils.logGasUsage("setup", setup)
+
+        assert.equal(await web3.eth.getBalance(gnosisSafe.address).toNumber(), web3.toWei(1, 'ether'))
+
+        let tx = await gnosisSafe.execTransaction(
+            executor, web3.toWei(1, 'ether'), "0x", CALL, 0, 0, 0, 0, 0, sigs, {from: executor}
+        )
+        utils.logGasUsage("execTransaction after setup", tx)
+
+        assert.equal(await web3.eth.getBalance(gnosisSafe.address).toNumber(), web3.toWei(0, 'ether'))
+
+    })
+})