From e4e276caf6c9517f7132024e038a6ce59f822048 Mon Sep 17 00:00:00 2001
From: Richard Meissner <richard@gnosis.io>
Date: Mon, 11 Nov 2019 15:00:25 +0100
Subject: [PATCH 1/2] :Don't allow transactions before setting up the Safe

---
 contracts/GnosisSafe.sol         | 10 +++++--
 test/gnosisSafeForceSafeSetup.js | 48 ++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)
 create mode 100644 test/gnosisSafeForceSafeSetup.js

diff --git a/contracts/GnosisSafe.sol b/contracts/GnosisSafe.sol
index bde712088..23bd3ce16 100644
--- a/contracts/GnosisSafe.sol
+++ b/contracts/GnosisSafe.sol
@@ -181,8 +181,12 @@ 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;
@@ -190,7 +194,7 @@ contract GnosisSafe
         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");
diff --git a/test/gnosisSafeForceSafeSetup.js b/test/gnosisSafeForceSafeSetup.js
new file mode 100644
index 000000000..11c4d6c9e
--- /dev/null
+++ b/test/gnosisSafeForceSafeSetup.js
@@ -0,0 +1,48 @@
+const utils = require('./utils/general')
+const safeUtils = require('./utils/execution')
+const BigNumber = require('bignumber.js')
+
+const GnosisSafe = artifacts.require("./GnosisSafe.sol")
+
+contract('GnosisSafe owner and module management', function(accounts) {
+
+    let gnosisSafe
+    let lw
+    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'))
+
+    })
+})

From fbbc712cf6d6bcd9dc4c9187c0f5abc01ff0c11a Mon Sep 17 00:00:00 2001
From: Richard Meissner <richard@gnosis.io>
Date: Mon, 11 Nov 2019 15:16:14 +0100
Subject: [PATCH 2/2] Cleanup

---
 test/gnosisSafeForceSafeSetup.js | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/test/gnosisSafeForceSafeSetup.js b/test/gnosisSafeForceSafeSetup.js
index 11c4d6c9e..35279af07 100644
--- a/test/gnosisSafeForceSafeSetup.js
+++ b/test/gnosisSafeForceSafeSetup.js
@@ -1,13 +1,10 @@
 const utils = require('./utils/general')
-const safeUtils = require('./utils/execution')
-const BigNumber = require('bignumber.js')
 
 const GnosisSafe = artifacts.require("./GnosisSafe.sol")
 
-contract('GnosisSafe owner and module management', function(accounts) {
+contract('GnosisSafe setup', function(accounts) {
 
     let gnosisSafe
-    let lw
     let executor = accounts[8]
 
     const CALL = 0