Fix bug allowing anyone to cancel an admin renounce (#4238)

Co-authored-by: Francisco Giordano <fg@frang.io>
pull/4244/head
Hadrien Croubois 2 years ago committed by GitHub
parent f355bd3a2a
commit 3ec4307c8a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 4
      contracts/access/AccessControlDefaultAdminRules.sol
  2. 24
      test/access/AccessControl.behavior.js

@ -106,7 +106,7 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
* non-administrated role. * non-administrated role.
*/ */
function renounceRole(bytes32 role, address account) public virtual override(AccessControl, IAccessControl) { function renounceRole(bytes32 role, address account) public virtual override(AccessControl, IAccessControl) {
if (role == DEFAULT_ADMIN_ROLE) { if (role == DEFAULT_ADMIN_ROLE && account == defaultAdmin()) {
(address newDefaultAdmin, uint48 schedule) = pendingDefaultAdmin(); (address newDefaultAdmin, uint48 schedule) = pendingDefaultAdmin();
require( require(
newDefaultAdmin == address(0) && _isScheduleSet(schedule) && _hasSchedulePassed(schedule), newDefaultAdmin == address(0) && _isScheduleSet(schedule) && _hasSchedulePassed(schedule),
@ -138,7 +138,7 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
* @dev See {AccessControl-_revokeRole}. * @dev See {AccessControl-_revokeRole}.
*/ */
function _revokeRole(bytes32 role, address account) internal virtual override { function _revokeRole(bytes32 role, address account) internal virtual override {
if (role == DEFAULT_ADMIN_ROLE && account == _currentDefaultAdmin) { if (role == DEFAULT_ADMIN_ROLE && account == defaultAdmin()) {
delete _currentDefaultAdmin; delete _currentDefaultAdmin;
} }
super._revokeRole(role, account); super._revokeRole(role, account);

@ -621,14 +621,15 @@ function shouldBehaveLikeAccessControlDefaultAdminRules(errorPrefix, delay, defa
}); });
describe('renounces admin', function () { describe('renounces admin', function () {
let expectedSchedule;
let delayPassed; let delayPassed;
let delayNotPassed;
beforeEach(async function () { beforeEach(async function () {
await this.accessControl.beginDefaultAdminTransfer(constants.ZERO_ADDRESS, { from: defaultAdmin }); await this.accessControl.beginDefaultAdminTransfer(constants.ZERO_ADDRESS, { from: defaultAdmin });
delayPassed = web3.utils expectedSchedule = web3.utils.toBN(await time.latest()).add(delay);
.toBN(await time.latest()) delayNotPassed = expectedSchedule;
.add(delay) delayPassed = expectedSchedule.addn(1);
.addn(1);
}); });
it('reverts if caller is not default admin', async function () { it('reverts if caller is not default admin', async function () {
@ -639,6 +640,15 @@ function shouldBehaveLikeAccessControlDefaultAdminRules(errorPrefix, delay, defa
); );
}); });
it("renouncing the admin role when not an admin doesn't affect the schedule", async function () {
await time.setNextBlockTimestamp(delayPassed);
await this.accessControl.renounceRole(DEFAULT_ADMIN_ROLE, other, { from: other });
const { newAdmin, schedule } = await this.accessControl.pendingDefaultAdmin();
expect(newAdmin).to.equal(constants.ZERO_ADDRESS);
expect(schedule).to.be.bignumber.equal(expectedSchedule);
});
it('keeps defaultAdmin consistent with hasRole if another non-defaultAdmin user renounces the DEFAULT_ADMIN_ROLE', async function () { it('keeps defaultAdmin consistent with hasRole if another non-defaultAdmin user renounces the DEFAULT_ADMIN_ROLE', async function () {
await time.setNextBlockTimestamp(delayPassed); await time.setNextBlockTimestamp(delayPassed);
@ -677,12 +687,6 @@ function shouldBehaveLikeAccessControlDefaultAdminRules(errorPrefix, delay, defa
}); });
describe('schedule not passed', function () { describe('schedule not passed', function () {
let delayNotPassed;
beforeEach(function () {
delayNotPassed = delayPassed.subn(1);
});
for (const [fromSchedule, tag] of [ for (const [fromSchedule, tag] of [
[-1, 'less'], [-1, 'less'],
[0, 'equal'], [0, 'equal'],

Loading…
Cancel
Save