diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fb0466f8d1..a9955665a2c 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/contracts/account/extensions/draft-AccountERC7579.sol b/contracts/account/extensions/draft-AccountERC7579.sol index 4d5a849f7d8..41ebf2fed20 100644 --- a/contracts/account/extensions/draft-AccountERC7579.sol +++ b/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; } diff --git a/test/account/extensions/AccountERC7579.behavior.js b/test/account/extensions/AccountERC7579.behavior.js index 7bf52f311aa..d531db7e046 100644 --- a/test/account/extensions/AccountERC7579.behavior.js +++ b/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'))