Skip to content

Commit 7f7cebd

Browse files
fix: STAA-15 remediation ii
1 parent 69a0663 commit 7f7cebd

File tree

4 files changed

+50
-3
lines changed

4 files changed

+50
-3
lines changed

src/StartaleSmartAccount.sol

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@ import {BaseAccount} from './core/BaseAccount.sol';
66
import {ERC7779Adapter} from './core/ERC7779Adapter.sol';
77
import {ExecutionHelper} from './core/ExecutionHelper.sol';
88
import {ModuleManager} from './core/ModuleManager.sol';
9+
10+
import {IERC7579Account} from './interfaces/IERC7579Account.sol';
911
import {IValidator} from './interfaces/IERC7579Module.sol';
1012
import {IStartaleSmartAccount} from './interfaces/IStartaleSmartAccount.sol';
13+
import {IAccountConfig} from './interfaces/core/IAccountConfig.sol';
1114
import {ExecutionLib} from './lib/ExecutionLib.sol';
1215
import {ACCOUNT_STORAGE_LOCATION} from './types/Constants.sol';
16+
import {IERC1271} from '@openzeppelin/contracts/interfaces/IERC1271.sol';
1317
import {IERC1155Receiver} from '@openzeppelin/contracts/token/ERC1155/IERC1155Receiver.sol';
1418
import {IERC721Receiver} from '@openzeppelin/contracts/token/ERC721/IERC721Receiver.sol';
1519
import {IERC165} from '@openzeppelin/contracts/utils/introspection/IERC165.sol';
@@ -203,7 +207,6 @@ contract StartaleSmartAccount is
203207
/// @dev This function can only be called by the EntryPoint or the account itself for security reasons.
204208
function installInterface(bytes4 interfaceId) external payable onlyEntryPointOrSelf {
205209
_installInterface(interfaceId);
206-
emit InterfaceInstalled(interfaceId);
207210
}
208211

209212
/// @notice Installs multiple interfaces to the smart account.
@@ -347,7 +350,7 @@ contract StartaleSmartAccount is
347350
_getPreValidationHook(MODULE_TYPE_PREVALIDATION_HOOK_ERC4337), MODULE_TYPE_PREVALIDATION_HOOK_ERC4337
348351
);
349352
_tryUninstallFallbacks();
350-
// Review: If we maintain banned selectors then clear them as well.
353+
_uninstallAllInterfaces();
351354
_initSentinelLists();
352355
}
353356

@@ -417,7 +420,9 @@ contract StartaleSmartAccount is
417420
AccountStorage storage ds = _getAccountStorage();
418421
if (
419422
interfaceId == type(IERC721Receiver).interfaceId || interfaceId == type(IERC1155Receiver).interfaceId
420-
|| interfaceId == type(IERC165).interfaceId
423+
|| interfaceId == type(IERC165).interfaceId || interfaceId == type(IERC1271).interfaceId
424+
|| interfaceId == type(IStartaleSmartAccount).interfaceId || interfaceId == type(IERC7579Account).interfaceId
425+
|| interfaceId == type(IAccountConfig).interfaceId
421426
) {
422427
return true;
423428
}

src/core/ModuleManager.sol

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,19 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
376376
delete ds.fallbackSelectors;
377377
}
378378

379+
/// @dev Uninstalls all interfaces from the smart account.
380+
/// @dev This function is called in the _onRedelegation function in StartaleSmartAccount.sol
381+
/// @notice clears all the storage variables related to interfaces.
382+
function _uninstallAllInterfaces() internal {
383+
AccountStorage storage ds = _getAccountStorage();
384+
uint256 len = ds.installedIfaces.length;
385+
for (uint256 i = 0; i < len; i++) {
386+
bytes4 interfaceId = ds.installedIfaces[i];
387+
_uninstallInterface(interfaceId);
388+
}
389+
delete ds.installedIfaces;
390+
}
391+
379392
/// @dev Sets the current hook in the storage to the specified address.
380393
/// @param hook The new hook address.
381394
function _setHook(address hook) internal virtual {
@@ -638,6 +651,7 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
638651
function _installInterface(bytes4 interfaceId) internal virtual {
639652
AccountStorage storage ds = _getAccountStorage();
640653
ds.supportedIfaces[interfaceId] = true;
654+
ds.installedIfaces.push(interfaceId);
641655
emit InterfaceInstalled(interfaceId);
642656
}
643657

@@ -646,6 +660,15 @@ abstract contract ModuleManager is AllStorage, EIP712, IModuleManager {
646660
function _uninstallInterface(bytes4 interfaceId) internal virtual {
647661
AccountStorage storage ds = _getAccountStorage();
648662
ds.supportedIfaces[interfaceId] = false;
663+
// Remove interfaceId from installedIfaces via swap-and-pop
664+
uint256 len = ds.installedIfaces.length;
665+
for (uint256 i = 0; i < len; i++) {
666+
if (ds.installedIfaces[i] == interfaceId) {
667+
ds.installedIfaces[i] = ds.installedIfaces[len - 1];
668+
ds.installedIfaces.pop();
669+
break;
670+
}
671+
}
649672
emit InterfaceUninstalled(interfaceId);
650673
}
651674

src/interfaces/core/IAllStorage.sol

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ interface IAllStorage {
2222
bytes4[] fallbackSelectors;
2323
///< ERC-165 interfaces installed on the account.
2424
mapping(bytes4 => bool) supportedIfaces;
25+
///< List of installed interfaces, initialized upon contract deployment.
26+
bytes4[] installedIfaces;
2527
///< Current hook module associated with this account.
2628
IHook hook;
2729
///< Mapping of hooks to requested timelocks.

test/foundry/unit/concrete/modulemanager/TestModuleManager_FallbackHandler.t.sol

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ pragma solidity ^0.8.29;
33

44
import {IModuleManager} from '../../../../../src/interfaces/core/IModuleManager.sol';
55

6+
import {StartaleSmartAccount} from '../../../../../src/StartaleSmartAccount.sol';
67
import {Counter} from '../../../mocks/Counter.sol';
78
import {MockHandler} from '../../../mocks/MockHandler.sol';
89
import '../../../mocks/MockNFT.sol';
@@ -368,6 +369,22 @@ contract TestModuleManager_FallbackHandler is TestModuleManagerBase {
368369
assertEq(chainId, block.chainid);
369370
assertEq(sender, expectedSender);
370371
assertEq(keccak256(resultData), keccak256(expectedData));
372+
373+
//Verify interface is not installed
374+
assertEq(BOB_ACCOUNT.supportsInterface(type(IHandler).interfaceId), false);
375+
376+
// Also register the interface. This could be done as a batch when installing the fallback handler.
377+
bytes memory callDataToInstallInterface =
378+
abi.encodeWithSelector(StartaleSmartAccount.installInterface.selector, type(IHandler).interfaceId);
379+
Execution[] memory executionToInstallInterface = new Execution[](1);
380+
executionToInstallInterface[0] = Execution(address(BOB_ACCOUNT), 0, callDataToInstallInterface);
381+
PackedUserOperation[] memory userOpsToInstallInterface = buildPackedUserOperation(
382+
BOB, BOB_ACCOUNT, EXECTYPE_DEFAULT, executionToInstallInterface, address(VALIDATOR_MODULE), 0
383+
);
384+
ENTRYPOINT.handleOps(userOpsToInstallInterface, payable(address(BOB.addr)));
385+
386+
// Verify the interface was installed
387+
assertEq(BOB_ACCOUNT.supportsInterface(type(IHandler).interfaceId), true);
371388
}
372389

373390
function test_ReturnBytes_and_Hook_fallback() public {

0 commit comments

Comments
 (0)