Prevent revert in isModuleInstalled for fallback modules on short additionalContext (#5961)

Co-authored-by: Hadrien Croubois <[email protected]>
Co-authored-by: ernestognw <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -3,6 +3,7 @@
 ### Bug fixes
 
 - `ERC165Checker`: Ensure the `supportsERC165` function returns false if the target reverts during the `supportsInterface(0xffffffff)` call. ([#5810](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/5880))
+- `AccountERC7579`: Prevent revert in `isModuleInstalled` for fallback modules when `additionalContext` has fewer than 4 bytes. The function now returns `false` instead of reverting, ensuring ERC-7579 compliance. ([#5961](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/5961))
 
 ### Breaking changes
 

contracts/account/extensions/draft-AccountERC7579.sol

@@ -149,7 +149,9 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75
     ) public view virtual returns (bool) {
         if (moduleTypeId == MODULE_TYPE_VALIDATOR) return _validators.contains(module);
         if (moduleTypeId == MODULE_TYPE_EXECUTOR) return _executors.contains(module);
-        if (moduleTypeId == MODULE_TYPE_FALLBACK) return _fallbacks[bytes4(additionalContext[0:4])] == module;
+        if (moduleTypeId == MODULE_TYPE_FALLBACK)
+            // ERC-7579 requires this function to return bool, never revert. Check length to avoid out-of-bounds access.
+            return additionalContext.length > 3 && _fallbacks[bytes4(additionalContext[0:4])] == module;
         return false;
     }
 

test/account/extensions/AccountERC7579.behavior.js

@@ -91,6 +91,22 @@ function shouldBehaveLikeAccountERC7579({ withHooks = false } = {}) {
       });
     });
 
+    describe('isModuleInstalled', function () {
+      it('should not revert if calldata is empty or too short', async function () {
+        await expect(
+          this.mock.isModuleInstalled(MODULE_TYPE_FALLBACK, this.modules[MODULE_TYPE_FALLBACK], '0x'),
+        ).to.eventually.equal(false);
+
+        await expect(
+          this.mock.isModuleInstalled(MODULE_TYPE_FALLBACK, this.modules[MODULE_TYPE_FALLBACK], '0x123456'),
+        ).to.eventually.equal(false);
+
+        await expect(
+          this.mock.isModuleInstalled(MODULE_TYPE_FALLBACK, this.modules[MODULE_TYPE_FALLBACK], '0x12345678'),
+        ).to.eventually.equal(false);
+      });
+    });
+
     describe('module installation', function () {
       it('should revert if the caller is not the canonical entrypoint or the account itself', async function () {
         await expect(this.mock.connect(this.other).installModule(MODULE_TYPE_VALIDATOR, this.mock, '0x'))