ERC7579: prevent installing/uninstalling without proper initData

Author: Amxx

certora/specs/Account.spec

@@ -205,7 +205,7 @@ invariant absentExecutorIsNotStored(address module, uint256 index)
 */
 // This guarantees that at most one fallback module is active for a given initData (i.e. selector)
 rule fallbackModule(address module, bytes initData) {
-    assert isModuleInstalled(FALLBACK_TYPE(), module, initData) <=> getFallbackHandler(getDataSelector(initData)) == module;
+    assert isModuleInstalled(FALLBACK_TYPE(), module, initData) <=> (initData.length >= 4 && getFallbackHandler(getDataSelector(initData)) == module);
 }
 
 rule moduleManagementRule(env e, method f, calldataarg args, uint256 moduleTypeId, address module, bytes additionalContext)

contracts/account/extensions/draft-AccountERC7579.sol

@@ -68,6 +68,12 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75
     /// @dev The account's {fallback} was called with a selector that doesn't have an installed handler.
     error ERC7579MissingFallbackHandler(bytes4 selector);
 
+    /// @dev The provided initData/deInitData for a fallback module is too short to extract a selector.
+    error ERC7579CannotDecodeFallbackData();
+
+    /// @dev The provided signature is not long enough to be parsed as a module signature.
+    error ERC7579InvalidModuleSignature();
+
     /// @dev Modifier that checks if the caller is an installed module of the given type.
     modifier onlyModule(uint256 moduleTypeId, bytes calldata additionalContext) {
         _checkModule(moduleTypeId, msg.sender, additionalContext);
@@ -384,7 +390,8 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75
     function _extractSignatureValidator(
         bytes calldata signature
     ) internal pure virtual returns (address module, bytes calldata innerSignature) {
-        return (address(bytes20(signature[0:20])), signature[20:]);
+        require(signature.length > 19, ERC7579InvalidModuleSignature());
+        return (address(bytes20(signature)), signature[20:]);
     }
 
     /**
@@ -399,6 +406,7 @@ abstract contract AccountERC7579 is Account, IERC1271, IERC7579Execution, IERC75
     function _decodeFallbackData(
         bytes memory data
     ) internal pure virtual returns (bytes4 selector, bytes memory remaining) {
+        require(data.length > 3, ERC7579CannotDecodeFallbackData());
         return (bytes4(data), data.slice(4));
     }
 

test/account/extensions/AccountERC7579.behavior.js

@@ -167,6 +167,17 @@ function shouldBehaveLikeAccountERC7579({ withHooks = false } = {}) {
         });
       }
 
+      it('should revert when installing a fallback module with an initData that is not long enough to encode a function selector', async function () {
+        const instance = this.modules[MODULE_TYPE_FALLBACK];
+        await expect(
+          this.mockFromEntrypoint.installModule(MODULE_TYPE_FALLBACK, instance, '0x'),
+        ).to.be.revertedWithCustomError(this.mock, 'ERC7579CannotDecodeFallbackData');
+
+        await expect(
+          this.mockFromEntrypoint.installModule(MODULE_TYPE_FALLBACK, instance, '0x123456'),
+        ).to.be.revertedWithCustomError(this.mock, 'ERC7579CannotDecodeFallbackData');
+      });
+
       withHooks &&
         describe('with hook', function () {
           beforeEach(async function () {
@@ -240,6 +251,17 @@ function shouldBehaveLikeAccountERC7579({ withHooks = false } = {}) {
         });
       }
 
+      it('should revert when uninstalling a fallback module with an initData that is not long enough to encode a function selector', async function () {
+        const instance = this.modules[MODULE_TYPE_FALLBACK];
+        await expect(
+          this.mockFromEntrypoint.uninstallModule(MODULE_TYPE_FALLBACK, instance, '0x'),
+        ).to.be.revertedWithCustomError(this.mock, 'ERC7579CannotDecodeFallbackData');
+
+        await expect(
+          this.mockFromEntrypoint.uninstallModule(MODULE_TYPE_FALLBACK, instance, '0x123456'),
+        ).to.be.revertedWithCustomError(this.mock, 'ERC7579CannotDecodeFallbackData');
+      });
+
       it('should revert uninstalling a module of type MODULE_TYPE_FALLBACK if a different module was installed for the provided selector', async function () {
         const instance = this.modules[MODULE_TYPE_FALLBACK];
         const anotherInstance = await ethers.deployContract('$ERC7579ModuleMock', [MODULE_TYPE_FALLBACK]);