Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 3 additions & 1 deletion contracts/account/extensions/draft-AccountERC7579.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
16 changes: 16 additions & 0 deletions test/account/extensions/AccountERC7579.behavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down
Loading