diff --git a/.changeset/fifty-owls-boil.md b/.changeset/fifty-owls-boil.md new file mode 100644 index 00000000000..ab53adc7c7c --- /dev/null +++ b/.changeset/fifty-owls-boil.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': patch +--- + +`AccountERC7579`: `isModuleInstalled` return false for address zero fallback & hook modules diff --git a/contracts/account/extensions/draft-AccountERC7579.sol b/contracts/account/extensions/draft-AccountERC7579.sol index 1e86b89a0bc..5400bc1fe10 100644 --- a/contracts/account/extensions/draft-AccountERC7579.sol +++ b/contracts/account/extensions/draft-AccountERC7579.sol @@ -154,7 +154,10 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75 if (moduleTypeId == MODULE_TYPE_EXECUTOR) return _executors.contains(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 + module != address(0) && + additionalContext.length > 3 && + _fallbacks[bytes4(additionalContext[0:4])] == module; return false; } diff --git a/contracts/account/extensions/draft-AccountERC7579Hooked.sol b/contracts/account/extensions/draft-AccountERC7579Hooked.sol index 98f61363114..7f3b3861488 100644 --- a/contracts/account/extensions/draft-AccountERC7579Hooked.sol +++ b/contracts/account/extensions/draft-AccountERC7579Hooked.sol @@ -64,7 +64,7 @@ abstract contract AccountERC7579Hooked is AccountERC7579 { bytes calldata data ) public view virtual override returns (bool) { return - (moduleTypeId == MODULE_TYPE_HOOK && module == hook()) || + (moduleTypeId == MODULE_TYPE_HOOK && module != address(0) && module == hook()) || super.isModuleInstalled(moduleTypeId, module, data); } diff --git a/test/account/extensions/AccountERC7579.behavior.js b/test/account/extensions/AccountERC7579.behavior.js index 94002158f96..e46c5345cd3 100644 --- a/test/account/extensions/AccountERC7579.behavior.js +++ b/test/account/extensions/AccountERC7579.behavior.js @@ -106,6 +106,16 @@ function shouldBehaveLikeAccountERC7579({ withHooks = false } = {}) { this.mock.isModuleInstalled(MODULE_TYPE_FALLBACK, this.modules[MODULE_TYPE_FALLBACK], '0x12345678'), ).to.eventually.equal(false); }); + + it('returns false for address(0) as module', async function () { + const moduleTypes = [MODULE_TYPE_FALLBACK, MODULE_TYPE_VALIDATOR, MODULE_TYPE_EXECUTOR]; + withHooks && moduleTypes.push(MODULE_TYPE_HOOK); + for (const moduleTypeId of moduleTypes) { + await expect(this.mock.isModuleInstalled(moduleTypeId, ethers.ZeroAddress, '0x12345678')).to.eventually.equal( + false, + ); + } + }); }); describe('module installation', function () {