From ad70057f7f2db8060644215c8bdbc16b5c8961ef Mon Sep 17 00:00:00 2001 From: hazim-j Date: Mon, 4 Aug 2025 09:44:30 +1000 Subject: [PATCH 1/2] Findings 20: change input to nodeHash in getRegisteredNode() --- snapshots/Keystore32NodeUCMT.json | 2 +- src/core/Keystore.sol | 4 ++-- src/interface/IKeystore.sol | 2 +- test/core/Keystore.t.sol | 36 +++++++++++++++++-------------- 4 files changed, 24 insertions(+), 20 deletions(-) diff --git a/snapshots/Keystore32NodeUCMT.json b/snapshots/Keystore32NodeUCMT.json index 67cf647..85af944 100644 --- a/snapshots/Keystore32NodeUCMT.json +++ b/snapshots/Keystore32NodeUCMT.json @@ -1,5 +1,5 @@ { - "1. registerNode": "76946", + "1. registerNode": "76859", "2. validate (with proof)": "12407", "3. validate (without proof)": "5284", "4. handleUpdates (with proof)": "64278", diff --git a/src/core/Keystore.sol b/src/core/Keystore.sol index 35ff02a..ecc309d 100644 --- a/src/core/Keystore.sol +++ b/src/core/Keystore.sol @@ -58,12 +58,12 @@ contract Keystore is IKeystore { _nodeCache[rootHash][nodeHash][msg.sender] = node; } - function getRegisteredNode(bytes32 refHash, address account, bytes calldata node) + function getRegisteredNode(bytes32 refHash, address account, bytes32 nodeHash) external view returns (bytes memory) { - return _nodeCache[_getCurrentRootHash(refHash, account)][keccak256(node)][account]; + return _nodeCache[_getCurrentRootHash(refHash, account)][nodeHash][account]; } function getRootHash(bytes32 refHash, address account) external view returns (bytes32 rootHash) { diff --git a/src/interface/IKeystore.sol b/src/interface/IKeystore.sol index 55bbd2c..c542c3f 100644 --- a/src/interface/IKeystore.sol +++ b/src/interface/IKeystore.sol @@ -18,7 +18,7 @@ interface IKeystore { function validate(ValidateAction calldata action) external view returns (uint256 validationData); function registerNode(bytes32 refHash, bytes32[] calldata proof, bytes calldata node) external; - function getRegisteredNode(bytes32 refHash, address account, bytes calldata node) + function getRegisteredNode(bytes32 refHash, address account, bytes32 nodeHash) external view returns (bytes memory); diff --git a/test/core/Keystore.t.sol b/test/core/Keystore.t.sol index 75244c0..656e445 100644 --- a/test/core/Keystore.t.sol +++ b/test/core/Keystore.t.sol @@ -48,9 +48,9 @@ contract KeystoreTest is Test { vm.assume(node.length >= 20 && bytes20(node) != 0); (bytes32 refHash, bytes memory proof) = _generateUCMT(nodes, index, node); - assertEq(keystore.getRegisteredNode(refHash, address(this), node).length, 0); + assertEq(keystore.getRegisteredNode(refHash, address(this), keccak256(node)).length, 0); _registerNode(refHash, proof, node); - assertGe(keystore.getRegisteredNode(refHash, address(this), node).length, 20); + assertGe(keystore.getRegisteredNode(refHash, address(this), keccak256(node)).length, 20); } function testFuzz_registerNodeWithMultipleRootHashUpdates( @@ -71,26 +71,27 @@ contract KeystoreTest is Test { assertEq(init.node, next.node); // Registers a proof when rootHash == refHash - assertEq(keystore.getRegisteredNode(init.root, address(this), init.node).length, 0); + bytes32 initNodeHash = keccak256(init.node); + assertEq(keystore.getRegisteredNode(init.root, address(this), initNodeHash).length, 0); _registerNode(init.root, init.proof, init.node); - assertGe(keystore.getRegisteredNode(init.root, address(this), init.node).length, 20); + assertGe(keystore.getRegisteredNode(init.root, address(this), initNodeHash).length, 20); // Update rootHash to nextHash - keystore.handleUpdates(_getUpdateActions(init.root, next.root, 0, "", abi.encode(keccak256(init.node)), data)); - assertEq(keystore.getRegisteredNode(init.root, address(this), init.node).length, 0); + keystore.handleUpdates(_getUpdateActions(init.root, next.root, 0, "", bytes.concat(initNodeHash), data)); + assertEq(keystore.getRegisteredNode(init.root, address(this), initNodeHash).length, 0); // Registers a proof when rootHash == nextHash _registerNode(init.root, next.proof, next.node); - assertGe(keystore.getRegisteredNode(init.root, address(this), init.node).length, 20); + assertGe(keystore.getRegisteredNode(init.root, address(this), initNodeHash).length, 20); // Update rootHash to finalHash // Note: if finalHash is zero, then we are essentially going back to the // refHash where the node is already cached. This is expected. - keystore.handleUpdates(_getUpdateActions(init.root, finalHash, 1, "", abi.encode(keccak256(next.node)), data)); + keystore.handleUpdates(_getUpdateActions(init.root, finalHash, 1, "", bytes.concat(keccak256(next.node)), data)); if (finalHash == 0) { - assertGe(keystore.getRegisteredNode(init.root, address(this), init.node).length, 20); + assertGe(keystore.getRegisteredNode(init.root, address(this), initNodeHash).length, 20); } else { - assertEq(keystore.getRegisteredNode(init.root, address(this), init.node).length, 0); + assertEq(keystore.getRegisteredNode(init.root, address(this), initNodeHash).length, 0); } } @@ -100,20 +101,22 @@ contract KeystoreTest is Test { vm.assume(node.length < 20); (bytes32 root, bytes memory proof) = _generateUCMT(nodes, index, node); - assertEq(keystore.getRegisteredNode(root, address(this), node).length, 0); + bytes32 nodeHash = keccak256(node); + assertEq(keystore.getRegisteredNode(root, address(this), nodeHash).length, 0); vm.expectRevert(IKeystore.InvalidNode.selector); _registerNode(root, proof, node); - assertEq(keystore.getRegisteredNode(root, address(this), node).length, 0); + assertEq(keystore.getRegisteredNode(root, address(this), nodeHash).length, 0); } function testFuzz_registerNodeWithInvalidVerifier(bytes32[] calldata nodes, uint256 index) public { bytes memory node = abi.encode(address(0)); (bytes32 root, bytes memory proof) = _generateUCMT(nodes, index, node); - assertEq(keystore.getRegisteredNode(root, address(this), node).length, 0); + bytes32 nodeHash = keccak256(node); + assertEq(keystore.getRegisteredNode(root, address(this), nodeHash).length, 0); vm.expectRevert(IKeystore.InvalidVerifier.selector); _registerNode(root, proof, node); - assertEq(keystore.getRegisteredNode(root, address(this), node).length, 0); + assertEq(keystore.getRegisteredNode(root, address(this), nodeHash).length, 0); } function testFuzz_registerNodeWithInvalidProof( @@ -125,10 +128,11 @@ contract KeystoreTest is Test { vm.assume(node.length >= 20 && bytes20(node) != 0); (bytes32 root,) = _generateUCMT(nodes, index, node); - assertEq(keystore.getRegisteredNode(root, address(this), node).length, 0); + bytes32 nodeHash = keccak256(node); + assertEq(keystore.getRegisteredNode(root, address(this), nodeHash).length, 0); vm.expectRevert(IKeystore.InvalidProof.selector); _registerNode(root, abi.encode(badProof), node); - assertEq(keystore.getRegisteredNode(root, address(this), node).length, 0); + assertEq(keystore.getRegisteredNode(root, address(this), nodeHash).length, 0); } function testFuzz_validate( From 55ffbd59a45f1f3dc046b407c9c92c31bdc0f044 Mon Sep 17 00:00:00 2001 From: hazim-j Date: Mon, 4 Aug 2025 09:46:37 +1000 Subject: [PATCH 2/2] dry --- test/core/Keystore.t.sol | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/core/Keystore.t.sol b/test/core/Keystore.t.sol index 656e445..7387fb3 100644 --- a/test/core/Keystore.t.sol +++ b/test/core/Keystore.t.sol @@ -48,9 +48,10 @@ contract KeystoreTest is Test { vm.assume(node.length >= 20 && bytes20(node) != 0); (bytes32 refHash, bytes memory proof) = _generateUCMT(nodes, index, node); - assertEq(keystore.getRegisteredNode(refHash, address(this), keccak256(node)).length, 0); + bytes32 nodeHash = keccak256(node); + assertEq(keystore.getRegisteredNode(refHash, address(this), nodeHash).length, 0); _registerNode(refHash, proof, node); - assertGe(keystore.getRegisteredNode(refHash, address(this), keccak256(node)).length, 20); + assertGe(keystore.getRegisteredNode(refHash, address(this), nodeHash).length, 20); } function testFuzz_registerNodeWithMultipleRootHashUpdates(