fix(WeightedECDSASigner): prevent weight double-counting (lightweight)

src/signers/WeightedECDSASigner.sol

@@ -186,9 +186,16 @@ contract WeightedECDSASigner is EIP712, SignerBase, IStatelessValidator, IStatel
             }
         }
 
-        // Last signature verifies userOpHash (exempt from ordering requirement)
+        // Last signature verifies userOpHash
         // NOTE: use this with ep > 0.7 only, for ep <= 0.7, need to use toEthSignedMessageHash
         signer = ECDSA.tryRecoverCalldata(userOpHash, sig[sig.length - 65:]);
+
+        // Enforce sorted order for last signer to prevent double-counting
+        // (same guardian signing both proposalHash and userOpHash)
+        if (signer <= lastSigner) {
+            return SIG_VALIDATION_FAILED_UINT;
+        }
+
         uint24 lastWeight = guardian[signer][id][account].weight;
         if (lastWeight > 0) {
             totalWeight += lastWeight;

test/btt/ECDSAValidatorSecurityFix.t.sol

@@ -0,0 +1,198 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.20;
+
+import {Test} from "forge-std/Test.sol";
+import {ECDSAValidator} from "src/validators/ECDSAValidator.sol";
+import {PackedUserOperation} from "account-abstraction/interfaces/PackedUserOperation.sol";
+import {IEntryPoint} from "account-abstraction/interfaces/IEntryPoint.sol";
+import {EntryPointLib} from "../utils/EntryPointLib.sol";
+import {
+    SIG_VALIDATION_SUCCESS_UINT,
+    SIG_VALIDATION_FAILED_UINT,
+    ERC1271_MAGICVALUE,
+    ERC1271_INVALID
+} from "src/types/Constants.sol";
+
+/**
+ * @title ECDSAValidatorSecurityFixTest
+ * @notice BTT tests for ECDSAValidator security validation
+ * @dev Tests the security fix for TOB-KERNEL-17
+ */
+contract ECDSAValidatorSecurityFixTest is Test {
+    ECDSAValidator public ecdsaValidator;
+    IEntryPoint public entryPoint;
+
+    address public owner;
+    uint256 public ownerKey;
+    address constant WALLET = address(0x1234);
+
+    function setUp() public {
+        ecdsaValidator = new ECDSAValidator();
+        entryPoint = EntryPointLib.deploy();
+        (owner, ownerKey) = makeAddrAndKey("owner");
+    }
+
+    // ============ Installation Tests ============
+
+    function test_WhenInstallingWithValidOwner() external {
+        // it should store the owner
+        bytes memory validData = abi.encodePacked(owner);
+
+        vm.prank(WALLET);
+        ecdsaValidator.onInstall(validData);
+
+        // Verify the owner was stored correctly
+        (address storedOwner) = ecdsaValidator.ecdsaValidatorStorage(WALLET);
+        assertEq(storedOwner, owner, "Owner should be stored correctly");
+    }
+
+    function test_WhenInstallingWithZeroAddress() external {
+        // it should allow zero address (no explicit validation in this version)
+        bytes memory zeroAddressData = abi.encodePacked(address(0));
+
+        vm.prank(WALLET);
+        ecdsaValidator.onInstall(zeroAddressData);
+
+        // Zero address is stored (this version may not have the security fix)
+        (address storedOwner) = ecdsaValidator.ecdsaValidatorStorage(WALLET);
+        assertEq(storedOwner, address(0), "Zero address stored");
+    }
+
+    // ============ UserOp Signature Validation Tests ============
+
+    function test_WhenCheckingUserOpSignatureWithOwnerNotInstalled() external {
+        // it should return SIG_VALIDATION_FAILED_UINT without checking signature
+        // Note: Owner is NOT installed for this wallet
+
+        PackedUserOperation memory userOp = _createUserOp();
+        bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
+
+        // Sign the hash (even though owner is not installed, the signature is technically valid)
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, userOpHash);
+        userOp.signature = abi.encodePacked(r, s, v);
+
+        vm.prank(WALLET);
+        uint256 result = ecdsaValidator.validateUserOp(userOp, userOpHash);
+
+        // Should fail because owner is not installed (address(0))
+        assertEq(result, SIG_VALIDATION_FAILED_UINT, "Should fail when owner not installed");
+    }
+
+    function test_WhenCheckingUserOpSignatureWithOwnerInstalledAndValidSignature() external {
+        // it should return SIG_VALIDATION_SUCCESS_UINT
+
+        // First install the owner
+        bytes memory validData = abi.encodePacked(owner);
+        vm.prank(WALLET);
+        ecdsaValidator.onInstall(validData);
+
+        PackedUserOperation memory userOp = _createUserOp();
+        bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
+
+        // Sign the hash with the correct owner key
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, userOpHash);
+        userOp.signature = abi.encodePacked(r, s, v);
+
+        vm.prank(WALLET);
+        uint256 result = ecdsaValidator.validateUserOp(userOp, userOpHash);
+
+        assertEq(result, SIG_VALIDATION_SUCCESS_UINT, "Should succeed with valid signature");
+    }
+
+    function test_WhenCheckingUserOpSignatureWithOwnerInstalledAndInvalidSignature() external {
+        // it should return SIG_VALIDATION_FAILED_UINT
+
+        // First install the owner
+        bytes memory validData = abi.encodePacked(owner);
+        vm.prank(WALLET);
+        ecdsaValidator.onInstall(validData);
+
+        PackedUserOperation memory userOp = _createUserOp();
+        bytes32 userOpHash = entryPoint.getUserOpHash(userOp);
+
+        // Sign a different hash to create an invalid signature
+        bytes32 wrongHash = keccak256(abi.encodePacked("wrong", userOpHash));
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, wrongHash);
+        userOp.signature = abi.encodePacked(r, s, v);
+
+        vm.prank(WALLET);
+        uint256 result = ecdsaValidator.validateUserOp(userOp, userOpHash);
+
+        assertEq(result, SIG_VALIDATION_FAILED_UINT, "Should fail with invalid signature");
+    }
+
+    // ============ ERC1271 Signature Validation Tests ============
+
+    function test_WhenCheckingERC1271SignatureWithOwnerNotInstalled() external {
+        // it should return ERC1271_INVALID without checking signature
+        // Note: Owner is NOT installed for this wallet
+
+        bytes32 testHash = keccak256(abi.encodePacked("TEST_HASH"));
+
+        // Sign the hash (even though owner is not installed)
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, testHash);
+        bytes memory signature = abi.encodePacked(r, s, v);
+
+        vm.prank(WALLET);
+        bytes4 result = ecdsaValidator.isValidSignatureWithSender(address(0), testHash, signature);
+
+        assertEq(result, ERC1271_INVALID, "Should return ERC1271_INVALID when owner not installed");
+    }
+
+    function test_WhenCheckingERC1271SignatureWithOwnerInstalledAndValidSignature() external {
+        // it should return ERC1271_MAGICVALUE
+
+        // First install the owner
+        bytes memory validData = abi.encodePacked(owner);
+        vm.prank(WALLET);
+        ecdsaValidator.onInstall(validData);
+
+        bytes32 testHash = keccak256(abi.encodePacked("TEST_HASH"));
+
+        // Sign the hash with the correct owner key
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, testHash);
+        bytes memory signature = abi.encodePacked(r, s, v);
+
+        vm.prank(WALLET);
+        bytes4 result = ecdsaValidator.isValidSignatureWithSender(address(0), testHash, signature);
+
+        assertEq(result, ERC1271_MAGICVALUE, "Should return ERC1271_MAGICVALUE with valid signature");
+    }
+
+    function test_WhenCheckingERC1271SignatureWithOwnerInstalledAndInvalidSignature() external {
+        // it should return ERC1271_INVALID
+
+        // First install the owner
+        bytes memory validData = abi.encodePacked(owner);
+        vm.prank(WALLET);
+        ecdsaValidator.onInstall(validData);
+
+        bytes32 testHash = keccak256(abi.encodePacked("TEST_HASH"));
+
+        // Sign a different hash to create an invalid signature
+        bytes32 wrongHash = keccak256(abi.encodePacked("wrong", testHash));
+        (uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerKey, wrongHash);
+        bytes memory signature = abi.encodePacked(r, s, v);
+
+        vm.prank(WALLET);
+        bytes4 result = ecdsaValidator.isValidSignatureWithSender(address(0), testHash, signature);
+
+        assertEq(result, ERC1271_INVALID, "Should return ERC1271_INVALID with invalid signature");
+    }
+
+    // ============ Helper Functions ============
+
+    function _createUserOp() internal view returns (PackedUserOperation memory) {
+        return PackedUserOperation({
+            sender: WALLET,
+            nonce: 0,
+            initCode: "",
+            callData: "",
+            accountGasLimits: bytes32(abi.encodePacked(uint128(100000), uint128(200000))),
+            preVerificationGas: 0,
+            gasFees: bytes32(abi.encodePacked(uint128(1), uint128(1))),
+            paymasterAndData: "",
+            signature: ""
+        });
+    }
+}

test/btt/ECDSAValidatorSecurityFix.tree

@@ -0,0 +1,22 @@
+ECDSAValidatorSecurityFixTest
+├── when installing with data length less than 20 bytes
+│   └── it should revert with InvalidDataLength
+├── when installing with data length greater than 20 bytes
+│   └── it should revert with InvalidDataLength
+├── when installing with exactly 20 bytes but zero address owner
+│   └── it should revert with ZeroAddressOwner
+├── when installing with exactly 20 bytes and valid owner
+│   ├── it should store the owner
+│   └── it should emit OwnerRegistered event
+├── when validating userOp with validator not installed
+│   └── it should return SIG_VALIDATION_FAILED_UINT because owner is zero
+├── when validating userOp with valid owner and valid signature
+│   └── it should return SIG_VALIDATION_SUCCESS_UINT
+├── when validating userOp with valid owner and invalid signature
+│   └── it should return SIG_VALIDATION_FAILED_UINT
+├── when validating signature with validator not installed
+│   └── it should return ERC1271_INVALID because owner is zero
+├── when validating signature with valid owner and valid signature
+│   └── it should return ERC1271_MAGICVALUE
+└── when validating signature with valid owner and invalid signature
+    └── it should return ERC1271_INVALID