From 5a77c9995fa125edc2f85879f0bd17c418ae225a Mon Sep 17 00:00:00 2001
From: Hadrien Croubois <hadrien.croubois@gmail.com>
Date: Thu, 7 Sep 2023 01:54:44 +0200
Subject: [PATCH] Make isConsumingScheduleOp return bytes4 to mitigate clashes
 (#4575)

Co-authored-by: Francisco Giordano <fg@frang.io>
---
 contracts/access/manager/AccessManaged.sol  | 4 ++--
 contracts/access/manager/AccessManager.sol  | 6 ++++--
 contracts/access/manager/IAccessManaged.sol | 2 +-
 contracts/access/manager/IAccessManager.sol | 3 ++-
 test/access/manager/AccessManager.test.js   | 2 +-
 5 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/contracts/access/manager/AccessManaged.sol b/contracts/access/manager/AccessManaged.sol
index 59265017a..70b6ecd74 100644
--- a/contracts/access/manager/AccessManaged.sol
+++ b/contracts/access/manager/AccessManaged.sol
@@ -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);
     }
 
     /**
diff --git a/contracts/access/manager/AccessManager.sol b/contracts/access/manager/AccessManager.sol
index be438536e..5261b1dd9 100644
--- a/contracts/access/manager/AccessManager.sol
+++ b/contracts/access/manager/AccessManager.sol
@@ -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);
             }
         }
 
diff --git a/contracts/access/manager/IAccessManaged.sol b/contracts/access/manager/IAccessManaged.sol
index 947e5bf5d..537a4749d 100644
--- a/contracts/access/manager/IAccessManaged.sol
+++ b/contracts/access/manager/IAccessManaged.sol
@@ -13,5 +13,5 @@ interface IAccessManaged {
 
     function setAuthority(address) external;
 
-    function isConsumingScheduledOp() external view returns (bool);
+    function isConsumingScheduledOp() external view returns (bytes4);
 }
diff --git a/contracts/access/manager/IAccessManager.sol b/contracts/access/manager/IAccessManager.sol
index 4f28baa7e..5906e9b23 100644
--- a/contracts/access/manager/IAccessManager.sol
+++ b/contracts/access/manager/IAccessManager.sol
@@ -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(
diff --git a/test/access/manager/AccessManager.test.js b/test/access/manager/AccessManager.test.js
index c157d6b7e..fc80c13ed 100644
--- a/test/access/manager/AccessManager.test.js
+++ b/test/access/manager/AccessManager.test.js
@@ -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,