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

Make isConsumingScheduleOp return bytes4 to mitigate clashes (#4575)

Browse Source 
Co-authored-by: Francisco Giordano <fg@frang.io>
This commit is contained in:
Hadrien Croubois
2023-09-07 01:54:44 +02:00
committed by GitHub
parent e2a9353dea
commit 5a77c9995f
5 changed files with 10 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
contracts/access/manager/AccessManaged.sol
View File

@ -85,8 +85,8 @@ abstract contract AccessManaged is Context, IAccessManaged {
* being consumed. Prevents denial of service for delayed restricted calls in the case that the contract performs
* attacker controlled calls.
*/
function isConsumingScheduledOp() public view returns (bool) {
return _consumingSchedule;
function isConsumingScheduledOp() public view returns (bytes4) {
return _consumingSchedule ? this.isConsumingScheduledOp.selector : bytes4(0);
}
/**

6
contracts/access/manager/AccessManager.sol
View File

@ -655,7 +655,9 @@ contract AccessManager is Context, Multicall, IAccessManager {
*/
function consumeScheduledOp(address caller, bytes calldata data) public virtual {
address target = _msgSender();
require(IAccessManaged(target).isConsumingScheduledOp());
if (IAccessManaged(target).isConsumingScheduledOp() != IAccessManaged.isConsumingScheduledOp.selector) {
revert AccessManagerUnauthorizedConsume(target);
}
_consumeScheduledOp(_hashOperation(caller, target, data));
}
@ -704,7 +706,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
(bool isAdmin, ) = hasGroup(ADMIN_GROUP, msgsender);
(bool isGuardian, ) = hasGroup(getGroupGuardian(getTargetFunctionGroup(target, selector)), msgsender);
if (!isAdmin && !isGuardian) {
revert AccessManagerCannotCancel(msgsender, caller, target, selector);
revert AccessManagerUnauthorizedCancel(msgsender, caller, target, selector);
}
}

2
contracts/access/manager/IAccessManaged.sol
View File

@ -13,5 +13,5 @@ interface IAccessManaged {
function setAuthority(address) external;
function isConsumingScheduledOp() external view returns (bool);
function isConsumingScheduledOp() external view returns (bytes4);
}

3
contracts/access/manager/IAccessManager.sol
View File

@ -48,7 +48,8 @@ interface IAccessManager {
error AccessManagerBadConfirmation();
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 AccessManagerUnauthorizedConsume(address target);
error AccessManagerUnauthorizedCancel(address msgsender, address caller, address target, bytes4 selector);
error AccessManagerInvalidInitialAdmin(address initialAdmin);
function canCall(

2
test/access/manager/AccessManager.test.js
View File

@ -961,7 +961,7 @@ contract('AccessManager', function (accounts) {
expect(await this.manager.getSchedule(this.opId)).to.not.be.bignumber.equal('0');
await expectRevertCustomError(this.cancel({ from: other }), 'AccessManagerCannotCancel', [
await expectRevertCustomError(this.cancel({ from: other }), 'AccessManagerUnauthorizedCancel', [
other,
user,
...this.call,