Merge pull request #20 from peppersec/audit-4

ether sending

contracts/ERC20Mixer.sol

@@ -26,12 +26,12 @@ contract ERC20Mixer is Mixer {
     token = _token;
   }
 
-  function _processDeposit() internal {
+  function _processDeposit() internal nonReentrant {
     require(msg.value == 0, "ETH value is supposed to be 0 for ETH mixer");
     _safeErc20TransferFrom(msg.sender, address(this), denomination);
   }
 
-  function _processWithdraw(address payable _recipient, address payable _relayer, uint256 _fee, uint256 _refund) internal {
+  function _processWithdraw(address payable _recipient, address payable _relayer, uint256 _fee, uint256 _refund) internal nonReentrant {
     require(msg.value == _refund, "Incorrect refund amount received by the contract");
 
     _safeErc20Transfer(_recipient, denomination - _fee);
@@ -39,7 +39,7 @@ contract ERC20Mixer is Mixer {
       _safeErc20Transfer(_relayer, _fee);
     }
     if (_refund > 0) {
-      _recipient.transfer(_refund);
+      _recipient.call.value(_refund)("");
     }
   }
 

contracts/ETHMixer.sol

@@ -22,18 +22,20 @@ contract ETHMixer is Mixer {
   ) Mixer(_verifier, _denomination, _merkleTreeHeight, _operator) public {
   }
 
-  function _processDeposit() internal {
+  function _processDeposit() internal nonReentrant {
     require(msg.value == denomination, "Please send `mixDenomination` ETH along with transaction");
   }
 
-  function _processWithdraw(address payable _recipient, address payable _relayer, uint256 _fee, uint256 _refund) internal {
+  function _processWithdraw(address payable _recipient, address payable _relayer, uint256 _fee, uint256 _refund) internal nonReentrant {
     // sanity checks
     require(msg.value == 0, "Message value is supposed to be zero for ETH mixer");
     require(_refund == 0, "Refund value is supposed to be zero for ETH mixer");
 
-    _recipient.transfer(denomination - _fee);
+    (bool success, ) = _recipient.call.value(denomination - _fee)("");
+    require(success, "payment to _recipient did not go thru");
     if (_fee > 0) {
-      _relayer.transfer(_fee);
+      (success, ) = _relayer.call.value(_fee)("");
+      require(success, "payment to _relayer did not go thru");
     }
   }
 }

contracts/Mixer.sol

@@ -12,12 +12,13 @@
 pragma solidity ^0.5.8;
 
 import "./MerkleTreeWithHistory.sol";
+import "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
 
 contract IVerifier {
   function verifyProof(bytes memory _proof, uint256[6] memory _input) public returns(bool);
 }
 
-contract Mixer is MerkleTreeWithHistory {
+contract Mixer is MerkleTreeWithHistory, ReentrancyGuard {
   uint256 public denomination;
   mapping(bytes32 => bool) public nullifierHashes;
   // we store all commitments just to prevent accidental deposits with the same commitment

package-lock.json

@@ -1680,6 +1680,113 @@
         "snarkjs": "^0.1.20",
         "typedarray-to-buffer": "^3.1.5",
         "web3": "^1.0.0-beta.55"
+      },
+      "dependencies": {
+        "ansi-regex": {
+          "version": "3.0.0",
+          "resolved": "https://registry.npmjs.org/ansi-regex/-/ansi-regex-3.0.0.tgz",
+          "integrity": "sha1-7QMXwyIGT3lGbAKWa922Bas32Zg="
+        },
+        "ansi-styles": {
+          "version": "3.2.1",
+          "resolved": "https://registry.npmjs.org/ansi-styles/-/ansi-styles-3.2.1.tgz",
+          "integrity": "sha512-VT0ZI6kZRdTh8YyJw3SMbYm/u+NqfsAxEpWO0Pf9sq8/e94WxxOpPKx9FR1FlyCtOVDNOQ+8ntlqFxiRc+r5qA==",
+          "requires": {
+            "color-convert": "^1.9.0"
+          }
+        },
+        "chalk": {
+          "version": "2.4.2",
+          "resolved": "https://registry.npmjs.org/chalk/-/chalk-2.4.2.tgz",
+          "integrity": "sha512-Mti+f9lpJNcwF4tWV8/OrTTtF1gZi+f8FqlyAdouralcFWFQWF2+NgCHShjkCb+IFBLq9buZwE1xckQU4peSuQ==",
+          "requires": {
+            "ansi-styles": "^3.2.1",
+            "escape-string-regexp": "^1.0.5",
+            "supports-color": "^5.3.0"
+          }
+        },
+        "eslint": {
+          "version": "5.16.0",
+          "resolved": "https://registry.npmjs.org/eslint/-/eslint-5.16.0.tgz",
+          "integrity": "sha512-S3Rz11i7c8AA5JPv7xAH+dOyq/Cu/VXHiHXBPOU1k/JAM5dXqQPt3qcrhpHSorXmrpu2g0gkIBVXAqCpzfoZIg==",
+          "requires": {
+            "@babel/code-frame": "^7.0.0",
+            "ajv": "^6.9.1",
+            "chalk": "^2.1.0",
+            "cross-spawn": "^6.0.5",
+            "debug": "^4.0.1",
+            "doctrine": "^3.0.0",
+            "eslint-scope": "^4.0.3",
+            "eslint-utils": "^1.3.1",
+            "eslint-visitor-keys": "^1.0.0",
+            "espree": "^5.0.1",
+            "esquery": "^1.0.1",
+            "esutils": "^2.0.2",
+            "file-entry-cache": "^5.0.1",
+            "functional-red-black-tree": "^1.0.1",
+            "glob": "^7.1.2",
+            "globals": "^11.7.0",
+            "ignore": "^4.0.6",
+            "import-fresh": "^3.0.0",
+            "imurmurhash": "^0.1.4",
+            "inquirer": "^6.2.2",
+            "js-yaml": "^3.13.0",
+            "json-stable-stringify-without-jsonify": "^1.0.1",
+            "levn": "^0.3.0",
+            "lodash": "^4.17.11",
+            "minimatch": "^3.0.4",
+            "mkdirp": "^0.5.1",
+            "natural-compare": "^1.4.0",
+            "optionator": "^0.8.2",
+            "path-is-inside": "^1.0.2",
+            "progress": "^2.0.0",
+            "regexpp": "^2.0.1",
+            "semver": "^5.5.1",
+            "strip-ansi": "^4.0.0",
+            "strip-json-comments": "^2.0.1",
+            "table": "^5.2.3",
+            "text-table": "^0.2.0"
+          }
+        },
+        "globals": {
+          "version": "11.12.0",
+          "resolved": "https://registry.npmjs.org/globals/-/globals-11.12.0.tgz",
+          "integrity": "sha512-WOBp/EEGUiIsJSp7wcv/y6MO+lV9UoncWqxuFfm8eBwzWNgyfBd6Gz+IeKQ9jCmyhoH99g15M3T+QaVHFjizVA=="
+        },
+        "semver": {
+          "version": "5.7.1",
+          "resolved": "https://registry.npmjs.org/semver/-/semver-5.7.1.tgz",
+          "integrity": "sha512-sauaDf/PZdVgrLTNYHRtpXa1iRiKcaebiKQ1BJdpQlWH2lCvexQdX55snPFyK7QzpudqbCI0qXFfOasHdyNDGQ=="
+        },
+        "snarkjs": {
+          "version": "0.1.20",
+          "resolved": "https://registry.npmjs.org/snarkjs/-/snarkjs-0.1.20.tgz",
+          "integrity": "sha512-tYmWiVm1sZiB44aIh5w/3HUaTntTUC4fv+CWs4rR0gfkt2KbHTpArOqZW++/Lxujrn9IypXVhdKVUr/eE6Hxfg==",
+          "requires": {
+            "big-integer": "^1.6.43",
+            "chai": "^4.2.0",
+            "escape-string-regexp": "^1.0.5",
+            "eslint": "^5.16.0",
+            "keccak": "^2.0.0",
+            "yargs": "^12.0.5"
+          }
+        },
+        "strip-ansi": {
+          "version": "4.0.0",
+          "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-4.0.0.tgz",
+          "integrity": "sha1-qEeQIusaw2iocTibY1JixQXuNo8=",
+          "requires": {
+            "ansi-regex": "^3.0.0"
+          }
+        },
+        "supports-color": {
+          "version": "5.5.0",
+          "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-5.5.0.tgz",
+          "integrity": "sha512-QjVjwdXIt408MIiAqCX4oUKsgU2EqAGzs2Ppkm4aQYbjm+ZEWEcW4SfFNTr4uMNZma0ey4f5lgLrkB0aX0QMow==",
+          "requires": {
+            "has-flag": "^3.0.0"
+          }
+        }
       }
     },
     "cli-cursor": {