From c6fb4c729017bfaded4038e33600613ca63515e7 Mon Sep 17 00:00:00 2001 From: Peter Emil Jensen Date: Sat, 9 Feb 2019 13:35:27 +0100 Subject: [PATCH] Fixed missing onlyProxy requirements for proxied pause, unpause and paused (#95) Also, added tests to catch the same error in the future. --- contracts/token/ETokenProxy.sol | 20 +++++++++++++++++--- test/token/EToken.test.js | 24 ++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/contracts/token/ETokenProxy.sol b/contracts/token/ETokenProxy.sol index 0b55e30..e4a9a5d 100644 --- a/contracts/token/ETokenProxy.sol +++ b/contracts/token/ETokenProxy.sol @@ -322,7 +322,11 @@ contract ETokenProxy is IETokenProxy, ETokenGuarded { /** Like EToken.pause but proxies calls as described in the documentation for the declaration of this contract. */ - function pauseProxy(address sender) external { + function pauseProxy(address sender) + external + isEnabled + onlyProxy + { if (isUpgraded()) { getUpgradedToken().pauseProxy(sender); } else { @@ -332,7 +336,11 @@ contract ETokenProxy is IETokenProxy, ETokenGuarded { /** Like EToken.unpause but proxies calls as described in the documentation for the declaration of this contract. */ - function unpauseProxy(address sender) external { + function unpauseProxy(address sender) + external + isEnabled + onlyProxy + { if (isUpgraded()) { getUpgradedToken().unpauseProxy(sender); } else { @@ -342,7 +350,13 @@ contract ETokenProxy is IETokenProxy, ETokenGuarded { /** Like EToken.paused but proxies calls as described in the documentation for the declaration of this contract. */ - function pausedProxy(address sender) external view returns (bool) { + function pausedProxy(address sender) + external + view + isEnabled + onlyProxy + returns (bool) + { if (isUpgraded()) { return getUpgradedToken().pausedProxy(sender); } else { diff --git a/test/token/EToken.test.js b/test/token/EToken.test.js index 912dcb6..ba1f184 100644 --- a/test/token/EToken.test.js +++ b/test/token/EToken.test.js @@ -1,4 +1,4 @@ -/* global artifacts, web3, contract */ +/* global artifacts, web3, contract, assert */ /* eslint-env mocha */ 'use strict'; @@ -39,9 +39,13 @@ const explicitSenderOps = [ ['transferFromProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]], ['increaseAllowanceProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]], ['decreaseAllowanceProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]], + ['mintProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]], ['burnProxy', [util.ZERO_ADDRESS, 0]], ['burnFromProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS, 0]], - ['changeMintingRecipientProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS]] + ['changeMintingRecipientProxy', [util.ZERO_ADDRESS, util.ZERO_ADDRESS]], + ['pauseProxy', [util.ZERO_ADDRESS]], + ['unpauseProxy', [util.ZERO_ADDRESS]], + ['pausedProxy', [util.ZERO_ADDRESS]] ]; const otherOps = [ @@ -372,6 +376,22 @@ contract('EToken', async function ( identifiesAsNewToken(); describe('Upgraded token rejects unauthorized for proxy sender functions', function () { + it('should test for all the proxy methods', async function () { + const methodsToTest = explicitSenderOps.map(o => o[0]).sort(); + const proxyMethods = Object.entries(this.token) + .filter( + ([key, value]) => key.endsWith('Proxy') && + typeof value === 'function' + ) + .map(o => o[0]) + .sort(); + + assert.deepEqual( + methodsToTest, + proxyMethods, + 'Not all proxy methods are tested'); + }); + explicitSenderOps.forEach(function (op) { it(`${op[0]} reverts`, async function () { await util.assertRevertsReason(upgradeToken[op[0]](...op[1], { from: owner }),