feat(rule-engine): add admin/owner-configurable on-chain maxRules cap (default 10), enforce limits + event/tests, and document rc3/nethermind finding #3 remediation

Author: rya-sge

CHANGELOG.md

@@ -45,10 +45,31 @@ forge lint
   - Update surya doc by running the 3 scripts in [./doc/script](./doc/script)
   - Update changelog
 
+### v3.0.0-rc3 - 2026-04-28
 
+### Security
+
+- Enforce an on-chain maximum rule count in `RulesManagementModule` to mitigate transfer liveness risk from unbounded per-transfer rule iteration (Nethermind AuditAgent finding 3 follow-up).
+- Add cap checks in both `addRule` and `setRules`, reverting with `RuleEngine_RulesManagementModule_MaxRulesExceeded(uint256)` when exceeded.
+
+### Added
+
+- Add `maxRules()` and `setMaxRules(uint256)` to `IRulesManagementModule`.
+- Add `DEFAULT_MAX_RULES = 10` and initialize module state with this default cap.
+- Add `SetMaxRules(uint256)` event emitted on cap updates.
+- Add dedicated access-control hook for cap governance:
+  - `RuleEngine`: `DEFAULT_ADMIN_ROLE` can update cap.
+  - `RuleEngineOwnable` and `RuleEngineOwnable2Step`: owner can update cap.
+
+### Testing
+
+- Add tests for default cap value, cap enforcement for `addRule` and `setRules`, and access control on `setMaxRules`.
+- Add event-emission coverage for `SetMaxRules`.
 
 ### v3.0.0-rc2 - 2026-04-14
 
+Commit: `ec4a24a96ca30e2ef8f79a06e49846a431e9b4b1`
+
 ### Dependencies
 
 - Update CMTAT submodule to [v3.2.0](https://github.com/CMTA/CMTAT/releases/tag/v3.2.0).

README.md

@@ -67,7 +67,7 @@ This diagram illustrates how a transfer with a CMTAT or ERC-3643 token with a Ru
 2. The transfer function inside the token calls the ERC-3643 function `transferred` from the RuleEngine with the following parameters inside: `from, to, value`.
 3. The Rule Engine calls each rule separately. If the transfer is not authorized by the rule, the rule must directly revert (no return value).
 
-> **Warning:** The RuleEngine iterates over all configured rules on every transfer (and on every call to `detectTransferRestriction`, `canTransfer`, etc.). Adding a large number of rules increases gas consumption for each transfer and may eventually exceed the block gas limit, effectively preventing any transfer from succeeding. There is no hard on-chain maximum rule count; administrators are responsible for sizing the rule set for their target blockchain and should keep it small. A misconfigured or gas-heavy rule can also impact all transfers.
+> **Warning:** The RuleEngine iterates over all configured rules on every transfer (and on every call to `detectTransferRestriction`, `canTransfer`, etc.). Adding a large number of rules increases gas consumption for each transfer and may eventually exceed the block gas limit, effectively preventing any transfer from succeeding. An on-chain rule cap is enforced (`maxRules`), set to `10` by default, and can be changed by governance (`DEFAULT_ADMIN_ROLE` on `RuleEngine`, owner on ownable variants). A misconfigured or gas-heavy rule can still impact all transfers.
 
 > **Warning (restriction code conventions):** Rule implementations should use unique ERC-1404 restriction codes across the rule set. If several rules intentionally share the same restriction code, they should return the exact same `messageForTransferRestriction` text for that code to avoid inconsistent operator/user feedback.
 
@@ -77,6 +77,7 @@ This diagram illustrates how a transfer with a CMTAT or ERC-3643 token with a Ru
 
 | RuleEngine version                                           | Compatible Versions                                          |
 | ------------------------------------------------------------ | ------------------------------------------------------------ |
+| **v3.0.0-rc3**                                               | CMTAT ≥ v3.0.0<br />CMTAT target version: [v3.2.0](https://github.com/CMTA/CMTAT/releases/tag/v3.2.0) |
 | **v3.0.0-rc2**                                               | CMTAT ≥ v3.0.0<br />CMTAT target version: [v3.2.0](https://github.com/CMTA/CMTAT/releases/tag/v3.2.0) |
 | **v3.0.0-rc1**                                               | CMTAT ≥ v3.0.0<br />CMTAT target version: [v3.2.0](https://github.com/CMTA/CMTAT/releases/tag/v3.2.0) |
 | **v3.0.0-rc0**                                               | CMTAT ≥ v3.0.0<br />                                         |

doc/security/audits/tools/nethermind-audit-agent/v3.0.0-rc1/audit_agent_report_1_v3.0.0-rc1-feedback.md

@@ -88,35 +88,45 @@ Code check confirmed:
 
 **File(s):** `src/modules/RulesManagementModule.sol`, `src/RuleEngineBase.sol`
 
-**Remediation commit:** `1caf4ea` — `Id-3: docs(rules-management): clarify operator-owned rule-count risk and blockchain-dependent gas limits`
+**Remediation commits:** `1caf4ea` (rc2 docs warning), `v3.0.0-rc3` hard-cap implementation
 
 ### Finding summary
 
 `addRule`/`setRules` have no cap on rule count. `_transferred` and `_detectTransferRestriction` loop over all rules per transfer. Adding too many rules can push gas cost above the block limit, permanently freezing token operations.
 
 ### Developer assessment
 
-**Partially agree.** The risk is real as an operational concern. However, a fixed on-chain maximum would be deployment-dependent and potentially overly restrictive across different blockchains and rule complexities. Operator responsibility is retained; the mitigation is explicit documentation.
+**Updated in rc3: fully addressed on-chain.** The prior rc2 docs-only mitigation was insufficient for liveness guarantees. `v3.0.0-rc3` introduces an on-chain configurable cap with a safe default.
 
 ### Implementation
 
-- **`src/modules/RulesManagementModule.sol`**:
-  - `setRules`: "No on-chain maximum number of rules is enforced. Operators are responsible for keeping the rule set size compatible with the target chain gas limits."
-  - `addRule`: "No on-chain maximum number of rules is enforced. Adding too many rules can increase transfer-time gas usage because rule checks are linear in rule count."
-  - `_transferred` (both overloads): "Complexity is O(number of configured rules). Large rule sets can make transfers too expensive on chains with lower block gas limits."
-- **`README.md`** — explicit "How it works" warning paragraph added about gas scaling and absence of an on-chain rule count cap.
-
-No runtime logic was changed.
+- **`src/modules/library/RulesManagementModuleInvariantStorage.sol`**
+  - Add `DEFAULT_MAX_RULES = 10`.
+  - Add `RuleEngine_RulesManagementModule_MaxRulesExceeded(uint256)`.
+  - Add `RuleEngine_RulesManagementModule_MaxRulesZeroNotAllowed()`.
+  - Add `SetMaxRules(uint256)` event.
+- **`src/interfaces/IRulesManagementModule.sol`**
+  - Add `maxRules()` and `setMaxRules(uint256)`.
+- **`src/modules/RulesManagementModule.sol`**
+  - Add `_maxRules` state initialized to default cap (`10`).
+  - Enforce cap in `addRule` and `setRules`.
+  - Add governance setter `setMaxRules(uint256)` with zero-value guard.
+- **Access control overrides**
+  - `RuleEngine`: cap setter restricted to `DEFAULT_ADMIN_ROLE`.
+  - `RuleEngineOwnable` / `RuleEngineOwnable2Step`: cap setter restricted to owner.
+- **`README.md`**
+  - Replace outdated “no on-chain cap” warning with current rc3 behavior.
 
 ### Verification
 
-Code check confirmed:
-- `RulesManagementModule.sol` line 46: `setRules` NatSpec — gas/cap warning present. ✓
-- `RulesManagementModule.sol` line 75: `addRule` NatSpec — linear gas warning present. ✓
-- `RulesManagementModule.sol` lines 169–173 and 188–192: `_transferred` overloads — O(n) and block-gas warning present. ✓
-- `README.md` line 69: gas-limit warning block present. ✓
+Code and tests confirm:
+- cap default is `10`;
+- `addRule` and `setRules` revert above cap;
+- cap update emits `SetMaxRules`;
+- cap setter is admin/owner restricted by variant;
+- zero cap is rejected.
 
-**Status: Implemented (warnings only — no hard cap by design).**
+**Status: Fixed in `v3.0.0-rc3` (runtime mitigation implemented).**
 
 ---
 

src/deployment/RuleEngine.sol

@@ -68,6 +68,7 @@ contract RuleEngine is ERC2771ModuleStandalone, RuleEngineBase, AccessControlEnu
     //////////////////////////////////////////////////////////////*/
     function _onlyComplianceManager() internal virtual override onlyRole(COMPLIANCE_MANAGER_ROLE) {}
     function _onlyRulesManager() internal virtual override onlyRole(RULES_MANAGEMENT_ROLE) {}
+    function _onlyRulesLimitManager() internal virtual override onlyRole(DEFAULT_ADMIN_ROLE) {}
 
     /**
      * @dev This surcharge is not necessary if you do not use the MetaTxModule

src/deployment/RuleEngineOwnable.sol

@@ -25,6 +25,7 @@ contract RuleEngineOwnable is RuleEngineOwnableShared, Ownable {
      * @dev Access control check using Ownable pattern
      */
     function _onlyRulesManager() internal virtual override onlyOwner {}
+    function _onlyRulesLimitManager() internal virtual override onlyOwner {}
 
     /**
      * @dev Access control check using Ownable pattern

src/deployment/RuleEngineOwnable2Step.sol

@@ -27,6 +27,7 @@ contract RuleEngineOwnable2Step is RuleEngineOwnableShared, Ownable2Step {
      * @dev Access control check using Ownable pattern
      */
     function _onlyRulesManager() internal virtual override onlyOwner {}
+    function _onlyRulesLimitManager() internal virtual override onlyOwner {}
 
     /**
      * @dev Access control check using Ownable pattern

src/interfaces/IRulesManagementModule.sol

@@ -6,6 +6,18 @@ pragma solidity ^0.8.20;
 import {IRule} from "./IRule.sol";
 
 interface IRulesManagementModule {
+    /**
+     * @notice Returns the maximum number of rules allowed in the engine.
+     */
+    function maxRules() external view returns (uint256);
+
+    /**
+     * @notice Updates the maximum number of rules allowed in the engine.
+     * @dev Access control is implementation specific (admin/owner).
+     * @param maxRules_ New maximum number of rules.
+     */
+    function setMaxRules(uint256 maxRules_) external;
+
     /**
      * @notice Defines the  rules for the rule engine.
      * @dev Sets the list of rule contract addresses for s.

src/modules/RulesManagementModule.sol

@@ -18,12 +18,19 @@ abstract contract RulesManagementModule is RulesManagementModuleInvariantStorage
         _;
     }
 
+    modifier onlyRulesLimitManager() {
+        _onlyRulesLimitManager();
+        _;
+    }
+
     /* ==== Type declaration === */
     using EnumerableSet for EnumerableSet.AddressSet;
 
     /* ==== State Variables === */
     /// @dev Array of rules
     EnumerableSet.AddressSet internal _rules;
+    /// @dev Maximum number of rules allowed in the engine.
+    uint256 internal _maxRules = DEFAULT_MAX_RULES;
 
     /*//////////////////////////////////////////////////////////////
                             PUBLIC/EXTERNAL FUNCTIONS
@@ -37,15 +44,16 @@ abstract contract RulesManagementModule is RulesManagementModuleInvariantStorage
      * Reverts if `rules_` is empty. Use {clearRules} to remove all rules explicitly.
      * To transition from one non-empty set to another without an enforcement gap,
      * call this function directly with the new set.
-     * No on-chain maximum number of rules is enforced. Operators are responsible
-     * for keeping the rule set size compatible with the target chain gas limits.
      * Security convention: rule contracts should be treated as trusted business logic,
      * but should not also be granted {RULES_MANAGEMENT_ROLE}.
      */
     function setRules(IRule[] calldata rules_) public virtual override(IRulesManagementModule) onlyRulesManager {
         if (rules_.length == 0) {
             revert RuleEngine_RulesManagementModule_ArrayIsEmpty();
         }
+        if (rules_.length > _maxRules) {
+            revert RuleEngine_RulesManagementModule_MaxRulesExceeded(_maxRules);
+        }
         if (_rules.length() > 0) {
             _clearRules();
         }
@@ -66,16 +74,36 @@ abstract contract RulesManagementModule is RulesManagementModuleInvariantStorage
 
     /**
      * @inheritdoc IRulesManagementModule
-     * @dev No on-chain maximum number of rules is enforced. Adding too many rules
-     * can increase transfer-time gas usage because rule checks are linear in rule count.
+     * @dev Reverts when the configured maximum number of rules is already reached.
      * Security convention: do not grant {RULES_MANAGEMENT_ROLE} to rule contracts.
      */
     function addRule(IRule rule_) public virtual override(IRulesManagementModule) onlyRulesManager {
+        if (_rules.length() >= _maxRules) {
+            revert RuleEngine_RulesManagementModule_MaxRulesExceeded(_maxRules);
+        }
         _checkRule(address(rule_));
         require(_rules.add(address(rule_)), RuleEngine_RulesManagementModule_OperationNotSuccessful());
         emit AddRule(rule_);
     }
 
+    /**
+     * @inheritdoc IRulesManagementModule
+     */
+    function maxRules() public view virtual override(IRulesManagementModule) returns (uint256) {
+        return _maxRules;
+    }
+
+    /**
+     * @inheritdoc IRulesManagementModule
+     */
+    function setMaxRules(uint256 maxRules_) public virtual override(IRulesManagementModule) onlyRulesLimitManager {
+        if (maxRules_ == 0) {
+            revert RuleEngine_RulesManagementModule_MaxRulesZeroNotAllowed();
+        }
+        _maxRules = maxRules_;
+        emit SetMaxRules(maxRules_);
+    }
+
     /**
      * @inheritdoc IRulesManagementModule
      */
@@ -197,4 +225,5 @@ abstract contract RulesManagementModule is RulesManagementModuleInvariantStorage
     }
 
     function _onlyRulesManager() internal virtual;
+    function _onlyRulesLimitManager() internal virtual;
 }

src/modules/library/RulesManagementModuleInvariantStorage.sol

@@ -5,12 +5,16 @@ pragma solidity ^0.8.20;
 import {IRule} from "../../interfaces/IRule.sol";
 
 abstract contract RulesManagementModuleInvariantStorage {
+    uint256 public constant DEFAULT_MAX_RULES = 10;
+
     /* ==== Errors === */
     error RuleEngine_RulesManagementModule_RuleAddressZeroNotAllowed();
     error RuleEngine_RulesManagementModule_RuleAlreadyExists();
     error RuleEngine_RulesManagementModule_RuleDoNotMatch();
     error RuleEngine_RulesManagementModule_ArrayIsEmpty();
     error RuleEngine_RulesManagementModule_OperationNotSuccessful();
+    error RuleEngine_RulesManagementModule_MaxRulesExceeded(uint256 maxRules);
+    error RuleEngine_RulesManagementModule_MaxRulesZeroNotAllowed();
 
     /* ============ Events ============ */
     /**
@@ -30,6 +34,12 @@ abstract contract RulesManagementModuleInvariantStorage {
      */
     event ClearRules();
 
+    /**
+     * @notice Emitted when the maximum allowed number of rules is updated.
+     * @param maxRules The new rule cap.
+     */
+    event SetMaxRules(uint256 maxRules);
+
     /* ==== Constant === */
     /// @notice Role to manage the ruleEngine
     // Will not be present in the final bytecode if not used

test/RuleEngine/AccessControl/RuleEngineAccessControl.sol

@@ -95,4 +95,23 @@ contract RuleEngineTest is Test, HelperContract {
         vm.expectRevert(ERC3643ComplianceModule.RuleEngine_ERC3643Compliance_UnauthorizedCaller.selector);
         ruleEngineMock.transferred(address(0), ADDRESS1, ADDRESS2, 10);
     }
+
+    function testCannotAttackerSetMaxRules() public {
+        vm.prank(ATTACKER);
+        vm.expectRevert(
+            abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, ATTACKER, bytes32(0))
+        );
+        ruleEngineMock.setMaxRules(12);
+    }
+
+    function testRulesManagerCannotSetMaxRulesWithoutAdminRole() public {
+        vm.prank(RULE_ENGINE_OPERATOR_ADDRESS);
+        ruleEngineMock.grantRole(RULES_MANAGEMENT_ROLE, WHITELIST_OPERATOR_ADDRESS);
+
+        vm.prank(WHITELIST_OPERATOR_ADDRESS);
+        vm.expectRevert(
+            abi.encodeWithSelector(AccessControlUnauthorizedAccount.selector, WHITELIST_OPERATOR_ADDRESS, bytes32(0))
+        );
+        ruleEngineMock.setMaxRules(12);
+    }
 }