From 926a4d7298e7aee011672d752bf705d08401a32c Mon Sep 17 00:00:00 2001 From: Alexey Date: Thu, 26 Sep 2019 18:46:49 +0300 Subject: [PATCH] relayhub protection --- cli.js | 7 +++++-- contracts/ETHMixer.sol | 7 +++++-- contracts/GSNMixer.sol | 3 ++- test/GSNsupport.test.js | 5 +++-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/cli.js b/cli.js index ad6e4fa..5d88b26 100755 --- a/cli.js +++ b/cli.js @@ -232,6 +232,8 @@ async function withdrawViaRelayer(note, receiver) { root: root, nullifierHash, receiver: bigInt(receiver), + relayer: bigInt(0), + fee: bigInt(web3.utils.toWei('0.01')), // Private snark inputs nullifier: deposit.nullifier, @@ -257,10 +259,11 @@ async function withdrawViaRelayer(note, receiver) { gasLimit: 5000000, verbose: true, } - // const provider = new GSNProvider('https://rinkeby.infura.io/v3/c7463beadf2144e68646ff049917b716', { signKey: account }) - const provider = new GSNDevProvider('http://localhost:8545', { signKey: account, HARDCODED_RELAYER_OPTS }) + const provider = new GSNProvider('https://rinkeby.infura.io/v3/c7463beadf2144e68646ff049917b716', { signKey: account }) + // const provider = new GSNDevProvider('http://localhost:8545', { signKey: account, HARDCODED_RELAYER_OPTS }) web3 = new Web3(provider) const netId = await web3.eth.net.getId() + console.log('netId', netId) // eslint-disable-next-line require-atomic-updates mixer = new web3.eth.Contract(contractJson.abi, contractJson.networks[netId].address) console.log('mixer address', contractJson.networks[netId].address) diff --git a/contracts/ETHMixer.sol b/contracts/ETHMixer.sol index 02a7136..ec24fac 100644 --- a/contracts/ETHMixer.sol +++ b/contracts/ETHMixer.sol @@ -34,9 +34,12 @@ contract ETHMixer is GSNMixer { require(msg.value == mixDenomination, "Please send `mixDenomination` ETH along with transaction"); } - event Debug(uint actualCharge, bytes context, address recipient); // this func is called by RelayerHub right after calling a target func function postRelayedCall(bytes memory context, bool /*success*/, uint actualCharge, bytes32 /*preRetVal*/) public onlyHub { + // this require allows to protect againt malicious relay hub that can drain the mixer + require(couldBeWithdrawn, "could be called only after withdrawViaRelayer"); + couldBeWithdrawn = false; + IRelayHub relayHub = IRelayHub(getHubAddr()); address payable recipient; uint256 nullifierHash; @@ -44,10 +47,10 @@ contract ETHMixer is GSNMixer { recipient := mload(add(context, 32)) nullifierHash := mload(add(context, 64)) } - emit Debug(actualCharge, context, recipient); recipient.transfer(mixDenomination - actualCharge); relayHub.depositFor.value(actualCharge)(address(this)); + emit Withdraw(recipient, nullifierHash, tx.origin, actualCharge); } } diff --git a/contracts/GSNMixer.sol b/contracts/GSNMixer.sol index 52ddfb0..5f4dcd4 100644 --- a/contracts/GSNMixer.sol +++ b/contracts/GSNMixer.sol @@ -14,6 +14,7 @@ contract GSNMixer is Mixer, GSNRecipient { ) Mixer(_verifier, _mixDenomination, _merkleTreeHeight, _emptyElement, _operator) public { } + bool couldBeWithdrawn; modifier onlyHub() { require(msg.sender == getHubAddr(), "only relay hub"); _; @@ -27,7 +28,7 @@ contract GSNMixer is Mixer, GSNRecipient { require(isKnownRoot(root), "Cannot find your merkle root"); // Make sure to use a recent one require(verifier.verifyProof(a, b, c, input), "Invalid withdraw proof"); nullifierHashes[nullifierHash] = true; - + couldBeWithdrawn = true; // we will process withdraw in postRelayedCall func } diff --git a/test/GSNsupport.test.js b/test/GSNsupport.test.js index 9b624b8..791fa0e 100644 --- a/test/GSNsupport.test.js +++ b/test/GSNsupport.test.js @@ -9,7 +9,7 @@ const Web3 = require('web3') const { toBN, toHex, toChecksumAddress } = require('web3-utils') const { takeSnapshot, revertSnapshot } = require('../lib/ganacheHelper') const { deployRelayHub, fundRecipient } = require('@openzeppelin/gsn-helpers') -const { GSNDevProvider } = require('@openzeppelin/gsn-provider') +const { GSNDevProvider, GSNProvider } = require('@openzeppelin/gsn-provider') const { ephemeral } = require('@openzeppelin/network') const Mixer = artifacts.require('./ETHMixer.sol') @@ -143,7 +143,8 @@ contract('GSN support', accounts => { isSpent.should.be.equal(false) const account = ephemeral() - const provider = new GSNDevProvider('http://localhost:8545', { + // const provider = new GSNProvider('https://kovan.infura.io/v3/c7463beadf2144e68646ff049917b716', { + const provider = new GSNProvider('http://localhost:8545', { signKey: account, ownerAddress, relayerAddress,