diff --git a/contracts/metatx/MinimalForwarder.sol b/contracts/metatx/MinimalForwarder.sol index c20125522b2..817a42cbd56 100644 --- a/contracts/metatx/MinimalForwarder.sol +++ b/contracts/metatx/MinimalForwarder.sol @@ -50,9 +50,17 @@ contract MinimalForwarder is EIP712 { (bool success, bytes memory returndata) = req.to.call{gas: req.gas, value: req.value}( abi.encodePacked(req.data, req.from) ); + // Validate that the relayer has sent enough gas for the call. // See https://ronan.eth.link/blog/ethereum-gas-dangers/ - assert(gasleft() > req.gas / 63); + if (gasleft() <= req.gas / 63) { + // We explicitly trigger invalid opcode to consume all gas and bubble-up the effects, since + // neither revert or assert consume all gas since Solidity 0.8.0 + // https://docs.soliditylang.org/en/v0.8.0/control-structures.html#panic-via-assert-and-error-via-require + assembly { + invalid() + } + } return (success, returndata); } diff --git a/test/metatx/MinimalForwarder.test.js b/test/metatx/MinimalForwarder.test.js index 6151f97ecf7..b8984e431e3 100644 --- a/test/metatx/MinimalForwarder.test.js +++ b/test/metatx/MinimalForwarder.test.js @@ -6,6 +6,7 @@ const { expectRevert, constants } = require('@openzeppelin/test-helpers'); const { expect } = require('chai'); const MinimalForwarder = artifacts.require('MinimalForwarder'); +const CallReceiverMock = artifacts.require('CallReceiverMock'); const name = 'MinimalForwarder'; const version = '0.0.1'; @@ -44,7 +45,7 @@ contract('MinimalForwarder', function (accounts) { nonce: Number(await this.forwarder.getNonce(this.sender)), data: '0x', }; - this.sign = ethSigUtil.signTypedMessage( + this.sign = () => ethSigUtil.signTypedMessage( this.wallet.getPrivateKey(), { data: { @@ -65,7 +66,7 @@ contract('MinimalForwarder', function (accounts) { }); it('success', async function () { - expect(await this.forwarder.verify(this.req, this.sign)).to.be.equal(true); + expect(await this.forwarder.verify(this.req, this.sign())).to.be.equal(true); }); afterEach(async function () { @@ -76,27 +77,27 @@ contract('MinimalForwarder', function (accounts) { context('invalid signature', function () { it('tampered from', async function () { - expect(await this.forwarder.verify({ ...this.req, from: accounts[0] }, this.sign)) + expect(await this.forwarder.verify({ ...this.req, from: accounts[0] }, this.sign())) .to.be.equal(false); }); it('tampered to', async function () { - expect(await this.forwarder.verify({ ...this.req, to: accounts[0] }, this.sign)) + expect(await this.forwarder.verify({ ...this.req, to: accounts[0] }, this.sign())) .to.be.equal(false); }); it('tampered value', async function () { - expect(await this.forwarder.verify({ ...this.req, value: web3.utils.toWei('1') }, this.sign)) + expect(await this.forwarder.verify({ ...this.req, value: web3.utils.toWei('1') }, this.sign())) .to.be.equal(false); }); it('tampered nonce', async function () { - expect(await this.forwarder.verify({ ...this.req, nonce: this.req.nonce + 1 }, this.sign)) + expect(await this.forwarder.verify({ ...this.req, nonce: this.req.nonce + 1 }, this.sign())) .to.be.equal(false); }); it('tampered data', async function () { - expect(await this.forwarder.verify({ ...this.req, data: '0x1742' }, this.sign)) + expect(await this.forwarder.verify({ ...this.req, data: '0x1742' }, this.sign())) .to.be.equal(false); }); it('tampered signature', async function () { - const tamperedsign = web3.utils.hexToBytes(this.sign); + const tamperedsign = web3.utils.hexToBytes(this.sign()); tamperedsign[42] ^= 0xff; expect(await this.forwarder.verify(this.req, web3.utils.bytesToHex(tamperedsign))) .to.be.equal(false); @@ -112,7 +113,7 @@ contract('MinimalForwarder', function (accounts) { }); it('success', async function () { - await this.forwarder.execute(this.req, this.sign); // expect to not revert + await this.forwarder.execute(this.req, this.sign()); // expect to not revert }); afterEach(async function () { @@ -124,36 +125,36 @@ contract('MinimalForwarder', function (accounts) { context('invalid signature', function () { it('tampered from', async function () { await expectRevert( - this.forwarder.execute({ ...this.req, from: accounts[0] }, this.sign), + this.forwarder.execute({ ...this.req, from: accounts[0] }, this.sign()), 'MinimalForwarder: signature does not match request', ); }); it('tampered to', async function () { await expectRevert( - this.forwarder.execute({ ...this.req, to: accounts[0] }, this.sign), + this.forwarder.execute({ ...this.req, to: accounts[0] }, this.sign()), 'MinimalForwarder: signature does not match request', ); }); it('tampered value', async function () { await expectRevert( - this.forwarder.execute({ ...this.req, value: web3.utils.toWei('1') }, this.sign), + this.forwarder.execute({ ...this.req, value: web3.utils.toWei('1') }, this.sign()), 'MinimalForwarder: signature does not match request', ); }); it('tampered nonce', async function () { await expectRevert( - this.forwarder.execute({ ...this.req, nonce: this.req.nonce + 1 }, this.sign), + this.forwarder.execute({ ...this.req, nonce: this.req.nonce + 1 }, this.sign()), 'MinimalForwarder: signature does not match request', ); }); it('tampered data', async function () { await expectRevert( - this.forwarder.execute({ ...this.req, data: '0x1742' }, this.sign), + this.forwarder.execute({ ...this.req, data: '0x1742' }, this.sign()), 'MinimalForwarder: signature does not match request', ); }); it('tampered signature', async function () { - const tamperedsign = web3.utils.hexToBytes(this.sign); + const tamperedsign = web3.utils.hexToBytes(this.sign()); tamperedsign[42] ^= 0xff; await expectRevert( this.forwarder.execute(this.req, web3.utils.bytesToHex(tamperedsign)), @@ -161,6 +162,23 @@ contract('MinimalForwarder', function (accounts) { ); }); }); + + it('bubble out of gas', async function () { + const receiver = await CallReceiverMock.new(); + const gasAvailable = 100000; + this.req.to = receiver.address; + this.req.data = receiver.contract.methods.mockFunctionOutOfGas().encodeABI(); + this.req.gas = 1000000; + + await expectRevert.assertion( + this.forwarder.execute(this.req, this.sign(), { gas: gasAvailable }), + ); + + const { transactions } = await web3.eth.getBlock('latest'); + const { gasUsed } = await web3.eth.getTransactionReceipt(transactions[0]); + + expect(gasUsed).to.be.equal(gasAvailable); + }); }); }); });