NoBey/openzeppelin-contracts
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
Co-authored-by: Francisco Giordano <fg@frang.io>
(cherry picked from commit 3ec4307c8a)
This commit is contained in:
Hadrien Croubois
2023-05-11 19:17:06 +02:00
committed by Francisco Giordano
parent 652ae921e1
commit e0fe936729
2 changed files with 16 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
contracts/access/AccessControlDefaultAdminRules.sol
View File

@ -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);

24
test/access/AccessControl.behavior.js
View File

@ -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'],