From 6ac3691f61e37607c05b4d97c1f9fa9c7f82ab8a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 4 Sep 2023 12:13:49 +0200 Subject: [PATCH 1/3] Prevent setting address(0) as the initialAdmin --- contracts/access/manager/AccessManager.sol | 4 ++++ contracts/access/manager/IAccessManager.sol | 1 + test/access/manager/AccessManager.test.js | 10 +++++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol index 01ac7664e00..db86fe0c072 100644 --- a/contracts/access/manager/AccessManager.sol +++ b/contracts/access/manager/AccessManager.sol @@ -112,6 +112,10 @@ contract AccessManager is Context, Multicall, IAccessManager { } constructor(address initialAdmin) { + if (initialAdmin == address(0)) { + revert AccessManagerInvalidInitialAdmin(address(0)); + } + // admin is active immediately and without any execution delay. _grantGroup(ADMIN_GROUP, initialAdmin, 0, 0); } diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol index f4a7da362ff..f16c08d23aa 100644 --- a/contracts/access/manager/IAccessManager.sol +++ b/contracts/access/manager/IAccessManager.sol @@ -52,6 +52,7 @@ interface IAccessManager { error AccessManagerUnauthorizedAccount(address msgsender, uint64 groupId); error AccessManagerUnauthorizedCall(address caller, address target, bytes4 selector); error AccessManagerCannotCancel(address msgsender, address caller, address target, bytes4 selector); + error AccessManagerInvalidInitialAdmin(address); function canCall( address caller, diff --git a/test/access/manager/AccessManager.test.js b/test/access/manager/AccessManager.test.js index 538120e00fe..20d5f6e2ac7 100644 --- a/test/access/manager/AccessManager.test.js +++ b/test/access/manager/AccessManager.test.js @@ -1,5 +1,5 @@ const { web3 } = require('hardhat'); -const { expectEvent, time } = require('@openzeppelin/test-helpers'); +const { constants, expectEvent, time } = require('@openzeppelin/test-helpers'); const { expectRevertCustomError } = require('../../helpers/customError'); const { selector } = require('../../helpers/methods'); const { clockFromReceipt } = require('../../helpers/time'); @@ -37,6 +37,14 @@ contract('AccessManager', function (accounts) { await this.manager.$_grantGroup(GROUPS.SOME, member, 0, 0); }); + it('rejects zero address for initialAdmin', async function () { + await expectRevertCustomError( + AccessManager.new(constants.ZERO_ADDRESS), + 'AccessManagerInvalidInitialAdmin', + [constants.ZERO_ADDRESS], + ); + }); + it('groups are correctly initialized', async function () { // group admin expect(await this.manager.getGroupAdmin(GROUPS.ADMIN)).to.be.bignumber.equal(GROUPS.ADMIN); From 348496f5937da5f9c962488be4f033fa2cc50f74 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Mon, 4 Sep 2023 12:19:00 +0200 Subject: [PATCH 2/3] fix lint --- test/access/manager/AccessManager.test.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/access/manager/AccessManager.test.js b/test/access/manager/AccessManager.test.js index 20d5f6e2ac7..5753cc1d423 100644 --- a/test/access/manager/AccessManager.test.js +++ b/test/access/manager/AccessManager.test.js @@ -38,11 +38,9 @@ contract('AccessManager', function (accounts) { }); it('rejects zero address for initialAdmin', async function () { - await expectRevertCustomError( - AccessManager.new(constants.ZERO_ADDRESS), - 'AccessManagerInvalidInitialAdmin', - [constants.ZERO_ADDRESS], - ); + await expectRevertCustomError(AccessManager.new(constants.ZERO_ADDRESS), 'AccessManagerInvalidInitialAdmin', [ + constants.ZERO_ADDRESS, + ]); }); it('groups are correctly initialized', async function () { From 68cd06bf5bd8329250e0a3dd47b98bdb788b1c14 Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Tue, 5 Sep 2023 10:23:01 +0200 Subject: [PATCH 3/3] Update contracts/access/manager/IAccessManager.sol Co-authored-by: Francisco --- contracts/access/manager/IAccessManager.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol index 4f33a4f06fb..63c2754317c 100644 --- a/contracts/access/manager/IAccessManager.sol +++ b/contracts/access/manager/IAccessManager.sol @@ -52,7 +52,7 @@ interface IAccessManager { error AccessManagerUnauthorizedAccount(address msgsender, uint64 groupId); error AccessManagerUnauthorizedCall(address caller, address target, bytes4 selector); error AccessManagerCannotCancel(address msgsender, address caller, address target, bytes4 selector); - error AccessManagerInvalidInitialAdmin(address); + error AccessManagerInvalidInitialAdmin(address initialAdmin); function canCall( address caller,