Merge pull request #92 from PracticalParticle/work

Work

Author: JaCoderX

contracts/core/lib/EngineBlox.sol

@@ -2116,6 +2116,10 @@ library EngineBlox {
         bytes4 handlerSelector,
         TxAction action
     ) internal view {
+        // Ensure both execution and handler selectors have registered function schemas
+        _validateFunctionSchemaExists(self, executionSelector);
+        _validateFunctionSchemaExists(self, handlerSelector);
+
         // Validate permission for the execution selector (underlying operation)
         if (!hasActionPermission(self, wallet, executionSelector, action)) {
             revert SharedValidation.NoPermission(wallet);

sdk/typescript/docs/runtime-rbac.md

@@ -299,7 +299,7 @@ Roles have maximum wallet limits:
 
 ```typescript
 const role = await runtimeRBAC.getRole(roleHash)
-const wallets = await runtimeRBAC.getWalletsInRole(roleHash)
+const wallets = await runtimeRBAC.getAuthorizedWallets(roleHash)
 
 if (wallets.length >= role.maxWallets) {
   throw new Error('Role has reached maximum wallet limit')
@@ -435,7 +435,7 @@ describe('RuntimeRBAC', () => {
   })
 
   it('should get wallets in role', async () => {
-    const wallets = await runtimeRBAC.getWalletsInRole(roleHash)
+    const wallets = await runtimeRBAC.getAuthorizedWallets(roleHash)
     expect(Array.isArray(wallets)).toBe(true)
   })
 })

test/foundry/docs/ATTACK_VECTORS_CODEX.md

@@ -2086,6 +2086,43 @@ removeFunctionSchema(selector, false);
 
 ---
 
+#### MEDIUM: Unsafe Function Unregistration with Stale Permissions
+- **ID**: `FS-004`
+- **Location**: `EngineBlox.sol:1185-1239`, `EngineBlox.sol:2104-2129`
+- **Severity**: MEDIUM
+- **Status**: ✅ **PROTECTED**
+
+**Description**:  
+Using `unregisterFunction(functionSelector, false)` to remove a function schema while roles still hold permissions for the selector, then attempting to create or execute new transactions using the stale selector.
+
+**Attack Scenario**:
+```solidity
+// 1. Register function schema and grant role permission for EXECUTE_TIME_DELAY_REQUEST
+registerFunction(schemaSelector, ...);
+addFunctionToRole(ROLE, FunctionPermission({ functionSelector: schemaSelector, ... }));
+
+// 2. Unsafely unregister schema while role still references selector
+unregisterFunction(schemaSelector, false); // safeRemoval = false
+
+// 3. Attempt to create or approve new transaction using stale selector
+txRequest(..., handlerSelector = schemaSelector, executionSelector = schemaSelector, ...); // Should fail
+```
+
+**Current Protection**:
+- ✅ `_validateFunctionSchemaExists` ensures a function schema exists for any selector being executed
+- ✅ `_validateExecutionAndHandlerPermissions` enforces schema existence for both `executionSelector` and `handlerSelector`
+- ✅ Requests and approvals using unregistered selectors revert with `ResourceNotFound`
+
+**Verification**:
+- Test unsafe unregistration followed by `txRequest` and approval flows
+- Verify that requests and approvals revert when function schema has been unregistered
+- Confirm that `safeRemoval = true` continues to enforce "no stale role permissions" invariant
+
+**Related Tests**:
+- `UnregisterFunctionFuzz.t.sol::testFuzz_UnsafeUnregisterPreventsNewRequests`
+
+---
+
 ## 12. Initialization & Upgrade
 
 ### 12.1 Initialization Attacks

test/foundry/docs/FINAL_COVERAGE_REPORT.md

@@ -1,14 +1,14 @@
 # Final Fuzz Test Coverage Report
 
-**Date**: February 21, 2026  
+**Date**: March 3, 2026  
 **Status**: ✅ **COMPREHENSIVE TEST SUITE COMPLETE**  
 **Goal**: 100% Coverage of All Attack Vectors
 
 ---
 
 ## Summary
 
-A comprehensive fuzz test suite covers **all 207+ attack vectors** identified in the security analysis, plus **21 protocol-vulnerabilities-index-derived vectors** (see [Attack Vectors Codex §18](./ATTACK_VECTORS_CODEX.md#18-protocol-vulnerabilities-index-derived-vectors)). The suite consists of **14 comprehensive fuzz files** with **148 tests**, plus additional fuzz, invariant, and unit tests: **37 test suites, 309 tests** (all passing as of last full run).
+A comprehensive fuzz test suite covers **all 207+ attack vectors** identified in the security analysis, plus **21 protocol-vulnerabilities-index-derived vectors** (see [Attack Vectors Codex §18](./ATTACK_VECTORS_CODEX.md#18-protocol-vulnerabilities-index-derived-vectors)). The suite consists of **14 comprehensive fuzz files** with **149 tests**, plus additional fuzz, invariant, and unit tests: **38 test suites, 308 tests** (all passing as of last full run, including `UnregisterFunctionFuzz.t.sol`).
 
 ---
 
@@ -97,8 +97,8 @@ A comprehensive fuzz test suite covers **all 207+ attack vectors** identified in
 - ✅ Malicious forwarder isolation
 - ✅ Gas exhaustion handling
 
-### Target Whitelist & Function Schema (6 vectors)
-- ✅ **100% coverage** - 8/8 tests passing
+### Target Whitelist & Function Schema (7 vectors)
+- ✅ **100% coverage** - 9/9 tests passing
 - ✅ Empty whitelist denial
 - ✅ Whitelist removal prevention
 - ✅ Handler selector validation
@@ -154,22 +154,22 @@ A comprehensive fuzz test suite covers **all 207+ attack vectors** identified in
 | Initialization | 3 | 9 | 100% |
 | Hook System | 4 | 2 | 100% |
 | Event Forwarding | 2 | 2 | 100% |
-| Whitelist/Schema | 6 | 8 | 100% |
+| Whitelist/Schema | 7 | 9 | 100% |
 | Time-Based | 3 | 1* | 100%* |
 | Role Management | 3 | 3* | 100%* |
 | Security Edge Cases | 10 | 10 | 100% |
 | Gas Exhaustion | 17 | 17 | 100% |
 | Protocol-Vulnerabilities-Index (§18) | 21 | 17 covered, 4 N/A | 100%* |
-| **TOTAL** | **207+** | **309 tests (37 suites)** | **100%** |
+| **TOTAL** | **207+** | **308 tests (38 suites)** | **100%** |
 
 *Covered in other test files. §18: 17 vectors covered by comprehensive suite; 4 N/A (no delegatecall/approve-before-call/proxy pattern).
 
 ---
 
 ## Test Execution Results
 
-### Current Status (February 2026)
-- ✅ **37 test suites**, **309 tests** (all passing; includes 14 comprehensive fuzz files with 148 tests)
+### Current Status (March 2026)
+- ✅ **38 test suites**, **308 tests** (all passing; includes 14 comprehensive fuzz files with 149 tests and additional targeted fuzz like `UnregisterFunctionFuzz.t.sol`)
 - ✅ **All critical attack vectors** covered
 - ✅ **All high-priority attack vectors** covered
 - ✅ **Protocol-vulnerabilities-index-derived vectors**: 17 covered/partial, 4 N/A — see [Codex §18](./ATTACK_VECTORS_CODEX.md#18-protocol-vulnerabilities-index-derived-vectors)
@@ -202,7 +202,7 @@ A comprehensive fuzz test suite covers **all 207+ attack vectors** identified in
 ### ✅ Complete Test Coverage Structure
 
 1. **14 comprehensive test files** organized by attack category
-2. **309 test functions** covering all attack vectors
+2. **308 test functions** covering all attack vectors
 3. **Direct mapping** to security analysis documents
 4. **Clear documentation** with execution guides
 5. **System safety limits** verified and tested

test/foundry/docs/TEST_DOCUMENTATION.md

@@ -1028,6 +1028,12 @@ This document provides comprehensive documentation of all test functions in the
 - **Purpose**: Fuzz tests for state machine workflows
 - **Status**: ✅ Tests passing
 
+#### UnregisterFunctionFuzz.t.sol
+- **Location**: `test/foundry/fuzz/UnregisterFunctionFuzz.t.sol`
+- **Purpose**: Fuzz tests for unsafe function schema unregistration (`unregisterFunction` with `safeRemoval = false`) and stale role permissions
+- **Covers**: [FS-004](./ATTACK_VECTORS_CODEX.md#medium-unsafe-function-unregistration-with-stale-permissions) – ensures requests using unregistered selectors revert via schema validation
+- **Status**: ✅ Tests passing
+
 #### ProtectedResourceFuzz.t.sol
 - **Location**: `test/foundry/fuzz/ProtectedResourceFuzz.t.sol`
 - **Purpose**: Fuzz tests for protected resources

test/foundry/docs/TEST_EXECUTION_GUIDE.md

@@ -10,7 +10,7 @@
 
 This guide provides comprehensive instructions for executing the Bloxchain Protocol test suite, including comprehensive fuzz tests, unit tests, integration tests, and security tests.
 
-**Test Suite Status**: ✅ **All tests passing** (37 test suites, 309 tests passed)
+**Test Suite Status**: ✅ **All tests passing** (38 test suites, 308 tests passed)
 
 ---
 
@@ -41,7 +41,7 @@ forge test --match-test "testFuzz_BatchOperationAtomicity" -vv
 ## Test Suite Structure
 
 ### Full suite summary
-- **37 test suites**, **309 tests** (all passing). Includes fuzz, unit, integration, and security tests.
+- **38 test suites**, **308 tests** (all passing). Includes fuzz, unit, integration, and security tests.
 
 ### Comprehensive Fuzz Tests (10 files – subset of full suite)
 

test/foundry/fuzz/ComprehensiveCompositeFuzz.t.sol

@@ -4,6 +4,8 @@ pragma solidity 0.8.34;
 import "../CommonBase.sol";
 import "../../../contracts/core/access/RuntimeRBAC.sol";
 import "../../../contracts/core/execution/GuardController.sol";
+import "../../../contracts/core/execution/interface/IGuardController.sol";
+import "../../../contracts/core/execution/lib/definitions/GuardControllerDefinitions.sol";
 import "../../../contracts/core/access/lib/definitions/RuntimeRBACDefinitions.sol";
 import "../../../contracts/core/lib/utils/SharedValidation.sol";
 import "../helpers/TestHelpers.sol";
@@ -32,8 +34,17 @@ contract ComprehensiveCompositeFuzzTest is CommonBase {
 
     function setUp() public override {
         super.setUp();
-        // Ensure mockTarget is whitelisted for executeWithTimeLock tests
-        // Note: This should be done in CommonBase, but we ensure it here for Composite tests
+        
+        // Register function schema for execute() on mockTarget and grant owner REQUEST+APPROVE
+        bytes4 executeSelector = bytes4(keccak256("execute()"));
+        EngineBlox.TxAction[] memory actions = new EngineBlox.TxAction[](2);
+        actions[0] = EngineBlox.TxAction.EXECUTE_TIME_DELAY_REQUEST;
+        actions[1] = EngineBlox.TxAction.EXECUTE_TIME_DELAY_APPROVE;
+        _registerFunction("execute()", "TEST_OPERATION", actions);
+        _grantOwnerPermission(executeSelector, actions);
+
+        // Whitelist mockTarget for executeWithTimeLock tests
+        _whitelistTarget(address(mockTarget), executeSelector);
     }
 
     /// @dev Converts uint to decimal string for deterministic role names (avoids vm.assume reject limit).
@@ -335,11 +346,19 @@ contract ComprehensiveCompositeFuzzTest is CommonBase {
         // Attempt to execute before time-lock expires
         if (block.timestamp < releaseTime) {
             vm.prank(broadcaster);
-            accountBlox.approveTimeLockExecutionWithMetaTx(metaTx);
-            
-            // Should fail - time-lock not expired
-            // Note: Meta-transaction approval still checks releaseTime
-            // This verifies time-lock applies to meta-transactions
+            try accountBlox.approveTimeLockExecutionWithMetaTx(metaTx) {
+                // If this ever succeeds before releaseTime, the underlying implementation is broken.
+                // Time-lock bypass via meta-transaction is a critical regression: fail the test immediately.
+                fail("approveTimeLockExecutionWithMetaTx succeeded before releaseTime");
+            } catch (bytes memory reason) {
+                bytes4 sel = bytes4(reason);
+                if (sel == SharedValidation.NoPermission.selector) {
+                    return; // Security guard: this path simply isn't authorized in this fuzz run
+                }
+                assembly {
+                    revert(add(reason, 0x20), mload(reason))
+                }
+            }
         }
         } catch (bytes memory reason) {
             bytes4 errorSelector = bytes4(reason);
@@ -570,4 +589,155 @@ contract ComprehensiveCompositeFuzzTest is CommonBase {
         }
         revert("No matching private key found");
     }
+
+    /**
+     * @dev Helper to register a function schema on accountBlox via GuardController guardConfigBatch
+     */
+    function _registerFunction(
+        string memory functionSignature,
+        string memory operationName,
+        EngineBlox.TxAction[] memory supportedActions
+    ) internal {
+        require(supportedActions.length > 0, "Supported actions cannot be empty");
+
+        IGuardController.GuardConfigAction[] memory actions = new IGuardController.GuardConfigAction[](1);
+        actions[0] = IGuardController.GuardConfigAction({
+            actionType: IGuardController.GuardConfigActionType.REGISTER_FUNCTION,
+            data: abi.encode(functionSignature, operationName, supportedActions)
+        });
+
+        bytes memory params = GuardControllerDefinitions.guardConfigBatchExecutionParams(actions);
+
+        EngineBlox.MetaTxParams memory metaTxParams = accountBlox.createMetaTxParams(
+            address(accountBlox),
+            GuardControllerDefinitions.GUARD_CONFIG_BATCH_META_SELECTOR,
+            EngineBlox.TxAction.SIGN_META_REQUEST_AND_APPROVE,
+            1 hours,
+            0,
+            owner
+        );
+
+        EngineBlox.MetaTransaction memory metaTx = accountBlox.generateUnsignedMetaTransactionForNew(
+            owner,
+            address(accountBlox),
+            0,
+            0,
+            GuardControllerDefinitions.CONTROLLER_OPERATION,
+            GuardControllerDefinitions.GUARD_CONFIG_BATCH_EXECUTE_SELECTOR,
+            params,
+            metaTxParams
+        );
+
+        uint256 signerPrivateKey = _getPrivateKeyForAddress(owner);
+        bytes32 messageHash = metaTx.message;
+        bytes32 ethSignedMessageHash = MessageHashUtils.toEthSignedMessageHash(messageHash);
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPrivateKey, ethSignedMessageHash);
+        metaTx.signature = abi.encodePacked(r, s, v);
+
+        vm.prank(broadcaster);
+        accountBlox.guardConfigBatchRequestAndApprove(metaTx);
+    }
+
+    /**
+     * @dev Helper to whitelist a target for a function selector on accountBlox
+     */
+    function _whitelistTarget(address target, bytes4 selector) internal {
+        IGuardController.GuardConfigAction[] memory actions = new IGuardController.GuardConfigAction[](1);
+        actions[0] = IGuardController.GuardConfigAction({
+            actionType: IGuardController.GuardConfigActionType.ADD_TARGET_TO_WHITELIST,
+            data: abi.encode(selector, target)
+        });
+
+        bytes memory params = GuardControllerDefinitions.guardConfigBatchExecutionParams(actions);
+
+        EngineBlox.MetaTxParams memory metaTxParams = accountBlox.createMetaTxParams(
+            address(accountBlox),
+            GuardControllerDefinitions.GUARD_CONFIG_BATCH_META_SELECTOR,
+            EngineBlox.TxAction.SIGN_META_REQUEST_AND_APPROVE,
+            1 hours,
+            0,
+            owner
+        );
+
+        EngineBlox.MetaTransaction memory metaTx = accountBlox.generateUnsignedMetaTransactionForNew(
+            owner,
+            address(accountBlox),
+            0,
+            0,
+            GuardControllerDefinitions.CONTROLLER_OPERATION,
+            GuardControllerDefinitions.GUARD_CONFIG_BATCH_EXECUTE_SELECTOR,
+            params,
+            metaTxParams
+        );
+
+        uint256 signerPrivateKey = _getPrivateKeyForAddress(owner);
+        bytes32 messageHash = metaTx.message;
+        bytes32 ethSignedMessageHash = MessageHashUtils.toEthSignedMessageHash(messageHash);
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(signerPrivateKey, ethSignedMessageHash);
+        metaTx.signature = abi.encodePacked(r, s, v);
+
+        vm.prank(broadcaster);
+        accountBlox.guardConfigBatchRequestAndApprove(metaTx);
+    }
+
+    /**
+     * @dev Helper to grant owner REQUEST+APPROVE permission for a function selector
+     */
+    function _grantOwnerPermission(bytes4 functionSelector, EngineBlox.TxAction[] memory actions) internal {
+        string memory roleName = "COMPOSITE_EXECUTE_ROLE";
+        bytes32 roleHash = keccak256(bytes(roleName));
+
+        // 1. Create role without permissions
+        IRuntimeRBAC.RoleConfigAction[] memory createActions = new IRuntimeRBAC.RoleConfigAction[](1);
+        EngineBlox.FunctionPermission[] memory emptyPermissions = new EngineBlox.FunctionPermission[](0);
+        createActions[0] = IRuntimeRBAC.RoleConfigAction({
+            actionType: IRuntimeRBAC.RoleConfigActionType.CREATE_ROLE,
+            data: abi.encode(roleName, 10, emptyPermissions)
+        });
+        bytes memory createParams = RuntimeRBACDefinitions.roleConfigBatchExecutionParams(abi.encode(createActions));
+        EngineBlox.MetaTransaction memory createMetaTx = _createMetaTxForRoleConfig(
+            owner,
+            createParams,
+            1 hours
+        );
+        vm.prank(broadcaster);
+        accountBlox.roleConfigBatchRequestAndApprove(createMetaTx);
+
+        // 2. Add owner wallet to the role
+        IRuntimeRBAC.RoleConfigAction[] memory addWalletActions = new IRuntimeRBAC.RoleConfigAction[](1);
+        addWalletActions[0] = IRuntimeRBAC.RoleConfigAction({
+            actionType: IRuntimeRBAC.RoleConfigActionType.ADD_WALLET,
+            data: abi.encode(roleHash, owner)
+        });
+        bytes memory addWalletParams = RuntimeRBACDefinitions.roleConfigBatchExecutionParams(abi.encode(addWalletActions));
+        EngineBlox.MetaTransaction memory addWalletMetaTx = _createMetaTxForRoleConfig(
+            owner,
+            addWalletParams,
+            1 hours
+        );
+        vm.prank(broadcaster);
+        accountBlox.roleConfigBatchRequestAndApprove(addWalletMetaTx);
+
+        // 3. Add function permission for the selector
+        bytes4[] memory handlerForSelectors = new bytes4[](1);
+        handlerForSelectors[0] = functionSelector;
+        EngineBlox.FunctionPermission memory permission = EngineBlox.FunctionPermission({
+            functionSelector: functionSelector,
+            grantedActionsBitmap: EngineBlox.createBitmapFromActions(actions),
+            handlerForSelectors: handlerForSelectors
+        });
+        IRuntimeRBAC.RoleConfigAction[] memory addPermActions = new IRuntimeRBAC.RoleConfigAction[](1);
+        addPermActions[0] = IRuntimeRBAC.RoleConfigAction({
+            actionType: IRuntimeRBAC.RoleConfigActionType.ADD_FUNCTION_TO_ROLE,
+            data: abi.encode(roleHash, permission)
+        });
+        bytes memory addPermParams = RuntimeRBACDefinitions.roleConfigBatchExecutionParams(abi.encode(addPermActions));
+        EngineBlox.MetaTransaction memory addPermMetaTx = _createMetaTxForRoleConfig(
+            owner,
+            addPermParams,
+            1 hours
+        );
+        vm.prank(broadcaster);
+        accountBlox.roleConfigBatchRequestAndApprove(addPermMetaTx);
+    }
 }