Merge pull request #20 from 0xdefaceme/better-exploitable

Improve exploitable and negotiator

contracts/Exploitable.sol

@@ -1,13 +1,17 @@
-pragma solidity 0.5.0;
+pragma solidity ^0.5.2;
 
 import "./Negotiator.sol";
+import 'openzeppelin-solidity/contracts/math/SafeMath.sol';
 
 contract Exploitable {
+    using SafeMath for uint256;
+
     mapping(address => uint256) public balances;
     address payable[] public investors;
 
     Negotiator public negotiator;
     address payable public owner;
+    uint256 public bounty;
 
     constructor(Negotiator _negotiator) public {
         owner = msg.sender;
@@ -20,7 +24,7 @@ contract Exploitable {
 
     function deposit() payable public {
         investors.push(msg.sender);
-        balances[msg.sender] += msg.value;    
+        balances[msg.sender] = balances[msg.sender].add(msg.value);
     }
 
     function withdraw() public { 
@@ -35,54 +39,53 @@ contract Exploitable {
         msg.sender.transfer(balances[msg.sender]);
     }
 
-    function pay(uint256 bountyId, string memory publicKey) public {
+    function increaseBounty() payable public {
         require(msg.sender == owner);
+        bounty = bounty.add(msg.value);
+    }
 
-        // Credit investors X% less as bounty is sent to zeroday
-        for (uint256 i = 0; i < investors.length; i++) {
-            uint256 balance = balances[investors[i]];
-            balances[investors[i]] *= balance * ((100 - percentageEIP1337()) / 100);
-        }
-        uint256 bounty = address(this).balance * (percentageEIP1337() / 100);
+    function decreaseBounty(uint256 amount) public {
+        require(msg.sender == owner);
+        bounty = bounty.sub(amount);
+        msg.sender.transfer(amount);
+    }
+
+    function pay(uint256 bountyId, string memory publicKey) public {
+        require(msg.sender == owner);
         negotiator.pay.value(bounty)(bountyId, publicKey);
+        bounty = bounty.sub(bounty);
     }
 
-    function decide(uint256 bountyId, bool decision) public {
+    function decide(
+        uint256 bountyId,
+        bool decision,
+        string memory reason
+    ) public {
         require(msg.sender == owner);
-        negotiator.decide(bountyId, decision);
+        negotiator.decide(bountyId, decision, reason);
     }
 
     function restore() payable public {
         require(tx.origin == owner);
         require(msg.sender == address(negotiator));
-
-        // Increase investors balances after vulnerability has been declined
-        for (uint256 i = 0; i < investors.length; i++) {
-            uint256 balance = balances[investors[i]];
-            balances[investors[i]] *= balance * ((percentageEIP1337() - 100) / 100);
-        }
+        bounty = bounty.add(msg.value);
     }
 
     function exit() public {
         require(tx.origin == owner);
         require(msg.sender == address(negotiator));
 
+        // TODO: Send to Claimable contract instead of paying out directly.
         for (uint256 i = 0; i < investors.length; i++) {
             uint256 balance = balances[investors[i]];
-            // Subtract bounty amount from balance of investors
-            uint256 balanceAfterBounty = balance * ((100 - percentageEIP1337()) / 100);
-
+            // decrease investor credit to zero
+            balances[investors[i]] = 0;
             // send non-blocking so that function doesn't fail
-            investors[i].send(balanceAfterBounty);
+            investors[i].send(balance);
         }
-        selfdestruct(owner);
     }
 
-    function implementsEIP1337() public pure returns (bool) {
+    function implementsExploitable() public pure returns (bool) {
         return true;
     }
-
-    function percentageEIP1337() public pure returns (uint256) {
-        return 10;
-    }
 }

contracts/IExploitable.sol

@@ -1,10 +1,9 @@
-pragma solidity 0.5.0;
+pragma solidity ^0.5.2;
 
 contract IExploitable {
-    function implementsEIP1337() public pure returns (bool);
-    function percentageEIP1337() public pure returns (uint256);
+    function implementsExploitable() public pure returns (bool);
     function pay(uint256 bountyId, string memory publicKey) public;
     function restore() public payable;
-    function decide(uint256 bountyId, bool decision) public;
+    function decide(uint256 bountyId, bool decision, string memory reason) public;
     function exit() public;
 }

contracts/Migrations.sol

@@ -1,4 +1,4 @@
-pragma solidity 0.5.0;
+pragma solidity ^0.5.2;
 
 contract Migrations {
   address public owner;

contracts/Negotiator.sol

@@ -1,4 +1,4 @@
-pragma solidity 0.5.0;
+pragma solidity ^0.5.2;
 
 import "./IExploitable.sol";
 
@@ -7,27 +7,30 @@ contract Negotiator {
     struct Vuln {
         IExploitable exploitable;
         address payable attacker;
-        uint256 damage;
         string key;
         uint256 bounty;
-        string hash;
+        string plain;
+        string encrypted;
         Status status;
+        string reason;
+        uint256 paidAt;
     }
 
-    enum Status {Commited, Paid, Revealed, Exited, Declined}
+    enum Status {Commited, Paid, Revealed, Exited, Declined, Timeout}
 
     Vuln[] public vulns;
+    uint256 public timeout = 1 days;
 
     event Commit(
         uint256 indexed id,
         address indexed exploitable,
-        uint256 indexed damage,
         address attacker
     );
 
     event Reveal(
         uint256 indexed id,
-        string indexed hash
+        string indexed plain,
+        string indexed encrypted
     );
 
     event Pay(
@@ -41,72 +44,88 @@ contract Negotiator {
         bool indexed exit
     );
 
-    // TODO: Decide what to do with this constructor
-    constructor() public {
-    }
-
     function commit(
-        IExploitable exploitable,
-        uint256 damage
+        IExploitable exploitable
     ) public returns (uint256 id) {
-        require(exploitable.implementsEIP1337());
-        require(damage <= address(exploitable).balance);
+        require(exploitable.implementsExploitable());
 
         id = vulns.push(Vuln({
             exploitable: exploitable,
             attacker: msg.sender,
-            damage: damage,
             key: "",
             bounty: 0,
-            hash: "",
-            status: Status.Commited
+            plain: "",
+            encrypted: "",
+            status: Status.Commited,
+            reason: "",
+            paidAt: 0
         })) - 1;
-        emit Commit(id, address(exploitable), damage, msg.sender);
-    }
-
-    function reveal(uint256 id, string memory hash) public {
-        Vuln storage vuln = vulns[id];
-        require(vuln.status == Status.Paid);
-        require(msg.sender == vuln.attacker);
-
-        vuln.hash = hash;
-        vuln.status = Status.Revealed;
-        emit Reveal(id, hash);
+        emit Commit(id, address(exploitable), msg.sender);
     }
 
     function pay(uint256 id, string memory key) public payable {
         Vuln storage vuln = vulns[id]; 
-        uint256 bounty = vuln.damage * (vuln.exploitable.percentageEIP1337() / 100);
-        require(msg.value >= bounty);
         require(msg.sender == address(vuln.exploitable));
         require(vuln.status == Status.Commited);
 
         vuln.key = key; 
+        vuln.paidAt = block.timestamp;
         vuln.bounty = msg.value;
         vuln.status = Status.Paid;
         emit Pay(id, key, msg.value);
     }
 
-    // TODO: Add string reason
-    // TODO: Decide should also work after a time out, in case the attacker
-    //       never reveals a secret
-    function decide(uint256 id, bool exit) public {
+    function reveal(
+        uint256 id,
+        string memory plain,
+        string memory encrypted
+    ) public {
+        Vuln storage vuln = vulns[id];
+        require(vuln.status == Status.Paid);
+        require(msg.sender == vuln.attacker);
+
+        vuln.plain = plain;
+        vuln.encrypted = encrypted;
+        vuln.status = Status.Revealed;
+        emit Reveal(id, plain, encrypted);
+    }
+
+    function decide(uint256 id, bool exit, string memory reason) public {
         Vuln storage vuln = vulns[id];
         require(msg.sender == address(vuln.exploitable));
-        require(vuln.status == Status.Revealed);
 
-        if (exit) {
-            vuln.status = Status.Exited;
-            vuln.exploitable.exit();
-            vuln.attacker.send(vuln.bounty);
-            emit Decide(id, true);
-        } else {
-            vuln.status = Status.Declined;
+        if (timedout(id)) {
+            vuln.status = Status.Timeout;
+            vuln.reason = "Attacker didn't reveal in time.";
             vuln.exploitable.restore.value(vuln.bounty)();
+            vuln.bounty = 0;
             emit Decide(id, false);
+        } else if (vuln.status == Status.Revealed) {
+            if (exit) {
+                vuln.status = Status.Exited;
+                vuln.reason = reason;
+                vuln.exploitable.exit();
+                vuln.attacker.send(vuln.bounty);
+                vuln.bounty = 0;
+                emit Decide(id, true);
+            } else {
+                vuln.status = Status.Declined;
+                vuln.reason = reason;
+                vuln.exploitable.restore.value(vuln.bounty)();
+                vuln.bounty = 0;
+                emit Decide(id, false);
+            }
+        } else {
+            revert();
         }
     }
 
+    function timedout(uint256 _id) public view returns (bool) {
+        Vuln storage vuln = vulns[_id];
+        return vuln.status == Status.Paid && 
+            vuln.paidAt + timeout > block.timestamp;
+    }
+
     function length() public view returns (uint256) {
         return vulns.length;
     }
@@ -130,9 +149,4 @@ contract Negotiator {
             }
         }
     }
-
-    function reward(uint256 id) public view returns (uint256) {
-        Vuln storage vuln = vulns[id]; 
-        return vuln.damage * (vuln.exploitable.percentageEIP1337() / 100);
-    }
 }

migrations/2_deploy_contracts.js

@@ -15,6 +15,9 @@ module.exports = function(deployer, network, accounts) {
           Exploitable.abi,
           instance.address
         );
+        await contract.methods
+          .increaseBounty()
+          .send({ from: accounts[0], value: web3.utils.toWei("0.5", "ether") });
         return contract.methods
           .deposit()
           .send({ from: accounts[0], value: web3.utils.toWei("1", "ether") });