Skip to content

Commit

Permalink
fix: edge case namespace merkle tree (#277)
Browse files Browse the repository at this point in the history 
<!--
Please read and fill out this form before submitting your PR.

Please make sure you have reviewed our contributors guide before
submitting your
first PR.
-->

## Overview

This fixes an edge case with the `verifyInner` for the NMT
implementation. It is the fix for `OS-CLST-ADV-01` from the Ottersec
audit.

Special thanks for Ottersec on providing the test cases and the fix.

## Checklist

<!-- 
Please complete the checklist to ensure that the PR is ready to be
reviewed.

IMPORTANT:
PRs should be left in Draft until the below checklist is completed.
-->

- [ ] New and updated code has appropriate documentation
- [ ] New and updated code has new and/or updated testing
- [ ] Required CI checks are passing
- [ ] Visual proof for any user facing features like CLI or
documentation updates
- [ ] Linked issues closed with keywords


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **Refactor**
- Improved the calculation logic for nodes in the Merkle Tree structure.
  
- **Tests**
- Added new tests to ensure the integrity of leaf and internal nodes in
the Merkle Tree.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
@rach-id
rach-id authored Jan 23, 2024
1 parent 8bbbfc5 commit 86cbb51
Show file tree
Hide file tree
Showing 2 changed files with 269 additions and 4 deletions.
4 changes: 2 additions & 2 deletions src/lib/tree/namespace/NamespaceMerkleTree.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ library NamespaceMerkleTree {
if (proof.sideNodes.length + heightOffset <= height - 1) {
return false;
}
if (proof.key - subTreeStartIndex < (1 << (height - heightOffset - 1))) {
if (proof.key - subTreeStartIndex < (1 << (height - 1))) {
node = nodeDigest(node, proof.sideNodes[height - heightOffset - 1]);
} else {
node = nodeDigest(proof.sideNodes[height - heightOffset - 1], node);
Expand All @@ -119,7 +119,7 @@ library NamespaceMerkleTree {
// is the case IFF 'stableEnd' (the last index of the largest full subtree)
// is equal to the number of leaves in the Merkle tree.
if (stableEnd != proof.numLeaves - 1) {
if (proof.sideNodes.length <= height - 1) {
if (proof.sideNodes.length <= height - heightOffset - 1) {
return false;
}
node = nodeDigest(node, proof.sideNodes[height - heightOffset - 1]);
Expand Down
269 changes: 267 additions & 2 deletions src/lib/tree/namespace/test/NamespaceMerkleTree.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,271 @@ contract NamespaceMerkleTreeTest is DSTest {
assertTrue(isValid);
}

function testVerifyLeafFiveOfEight() external {
NamespaceNode memory root = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040),
0x34e6541306dc4e57a5a2a9ef57a46d5705ed09efb8c6a02580d3a972922b6862
);
NamespaceNode[] memory sideNodes = new NamespaceNode[](3);
sideNodes[0] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x671157a4e268f7060abbdc4b48f091589555a0775a2694e6899833ec98fdb296
);
sideNodes[1] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x1b79ffd74644e8c287fe5f1dd70bc8ea02738697cebf2810ffb2dc5157485c40
);
sideNodes[2] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040),
0xa8dcd9f365fb64aa6d72b5027fe74db0fc7d009c2d75c7b9b9656927281cb35e
);

uint256 key = 4;
uint256 numLeaves = 8;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bytes memory data = hex"05";
bool isValid = NamespaceMerkleTree.verify(root, proof, PARITY_SHARE_NAMESPACE(), data);
assertTrue(isValid);
}

function testVerifyLeafFourOfEight() external {
NamespaceNode memory root = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040),
0x34e6541306dc4e57a5a2a9ef57a46d5705ed09efb8c6a02580d3a972922b6862
);
NamespaceNode[] memory sideNodes = new NamespaceNode[](3);
sideNodes[0] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000030),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000030),
0x35e864d3e196ef0986fcf18eea2782c7e68794c7106dacc2a4f7e40d6d7c7069
);
sideNodes[1] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000020),
0x1dae5c3d39a8bf31ea33ba368238a52f816cd50485c580565609554cf360c91f
);
sideNodes[2] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x5aa3e7ea31995fdd38f41015275229b290a8ee4810521db766ad457b9a8373d6
);

uint256 key = 3;
uint256 numLeaves = 8;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bytes memory data = hex"04";
bool isValid = NamespaceMerkleTree.verify(
root, proof, Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040), data
);
assertTrue(isValid);
}

function testVerifyLeafThreeOfEight() external {
NamespaceNode memory root = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040),
0x34e6541306dc4e57a5a2a9ef57a46d5705ed09efb8c6a02580d3a972922b6862
);
NamespaceNode[] memory sideNodes = new NamespaceNode[](3);
sideNodes[0] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040),
0xecdeb08b04dd92a17fec560e20c53269f65beff5a2626fa64f61bfa45b09119d
);
sideNodes[1] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000020),
0x1dae5c3d39a8bf31ea33ba368238a52f816cd50485c580565609554cf360c91f
);
sideNodes[2] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x5aa3e7ea31995fdd38f41015275229b290a8ee4810521db766ad457b9a8373d6
);

uint256 key = 2;
uint256 numLeaves = 8;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bytes memory data = hex"03";
bool isValid = NamespaceMerkleTree.verify(
root, proof, Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000030), data
);
assertTrue(isValid);
}

function testVerifyLeafFiveOfSeven() external {
NamespaceNode memory root = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040),
0xfe7100a7170cba2065c48e01cb18772ed93865100bb7610aed3f614829c87a48
);
NamespaceNode[] memory sideNodes = new NamespaceNode[](3);
sideNodes[0] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x671157a4e268f7060abbdc4b48f091589555a0775a2694e6899833ec98fdb296
);
sideNodes[1] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x2669e36b48e95bd9903300e50c27c53984fc439f6235fade08e3f14e78a42aac
);
sideNodes[2] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040),
0xa8dcd9f365fb64aa6d72b5027fe74db0fc7d009c2d75c7b9b9656927281cb35e
);

uint256 key = 4;
uint256 numLeaves = 7;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bytes memory data = hex"05";
bool isValid = NamespaceMerkleTree.verify(root, proof, PARITY_SHARE_NAMESPACE(), data);
assertTrue(isValid);
}

function testVerifyLeafNineOfTen() external {
NamespaceNode memory root = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000060),
0x21013157ca1c0d454c988665e05894f5cf9422928552349ac3fd359bd1d39ac1
);
NamespaceNode[] memory sideNodes = new NamespaceNode[](2);
sideNodes[0] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x8ecd4167595d96b6caf19871584b07f255a4d80037b122c9f1f71acb1366a1ae
);
sideNodes[1] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000060),
0xee695202b2d3090a2319e7491483cf50e71a5907cebcf1fed4d02daa02f39827
);

uint256 key = 8;
uint256 numLeaves = 10;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bytes memory data = hex"09";
bool isValid = NamespaceMerkleTree.verify(root, proof, PARITY_SHARE_NAMESPACE(), data);
assertTrue(isValid);
}

function testVerifyLeafTwelveOfThirteen() external {
NamespaceNode memory root = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000060),
0xcdf9d9d4b408a7bf1ec5653dcb5f8cda23a329754890b63344e706302ef70e43
);
NamespaceNode[] memory sideNodes = new NamespaceNode[](4);
sideNodes[0] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x311733a16ba3f14dca59dcd88e6b64276613cac5a9e20a4b228c520722851b3a
);
sideNodes[1] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x8137f8ca69ccd4d39d47836ace7aa22b010222eaa904e67a6ff9bf05542f7124
);
sideNodes[2] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x3666000822ff8e0e5bf01c170264fe39dc38d887a5ec5e87b4f72b328a323ec5
);
sideNodes[3] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000060),
0xee695202b2d3090a2319e7491483cf50e71a5907cebcf1fed4d02daa02f39827
);

uint256 key = 11;
uint256 numLeaves = 13;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bytes memory data = hex"0c";
bool isValid = NamespaceMerkleTree.verify(root, proof, PARITY_SHARE_NAMESPACE(), data);
assertTrue(isValid);
}

function testVerifyLeafThirteenOfThirteen() external {
NamespaceNode memory root = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000060),
0xcdf9d9d4b408a7bf1ec5653dcb5f8cda23a329754890b63344e706302ef70e43
);
NamespaceNode[] memory sideNodes = new NamespaceNode[](2);
sideNodes[0] = NamespaceNode(
PARITY_SHARE_NAMESPACE(),
PARITY_SHARE_NAMESPACE(),
0x31501dc8a114b0aa3cde0f4f99f0643760b3b11303ab1ee568538f3e5769fbfe
);
sideNodes[1] = NamespaceNode(
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010),
Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000060),
0xee695202b2d3090a2319e7491483cf50e71a5907cebcf1fed4d02daa02f39827
);

uint256 key = 12;
uint256 numLeaves = 13;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bytes memory data = hex"0d";
bool isValid = NamespaceMerkleTree.verify(root, proof, PARITY_SHARE_NAMESPACE(), data);
assertTrue(isValid);
}

function testVerifyInternalNodeOneAndTwoOfFour() external pure {
uint256 key = 1;
uint256 numLeaves = 4;
uint256 startingHeight = 2;
NamespaceNode[] memory sideNodes = new NamespaceNode[](1);

bytes memory data = hex"01";
NamespaceNode memory node1 =
leafDigest(Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010), data);
NamespaceNode memory node2 =
leafDigest(Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000020), data);
NamespaceNode memory node3 =
leafDigest(Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000030), data);
NamespaceNode memory node4 =
leafDigest(Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000040), data);
NamespaceNode memory node1_2 = nodeDigest(node1, node2);
NamespaceNode memory node3_4 = nodeDigest(node3, node4);
NamespaceNode memory root = nodeDigest(node1_2, node3_4);
NamespaceNode memory startingNode = node1_2;

sideNodes[0] = node3_4;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bool isValid = NamespaceMerkleTree.verifyInner(root, proof, startingNode, startingHeight);
assert(isValid);
}

function testVerifyInternalNodeOneAndTwoOfThree() external pure {
uint256 key = 0;
uint256 numLeaves = 3;
uint256 startingHeight = 2;
NamespaceNode[] memory sideNodes = new NamespaceNode[](1);

bytes memory data = hex"01";
NamespaceNode memory node1 =
leafDigest(Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000010), data);
NamespaceNode memory node2 =
leafDigest(Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000020), data);
NamespaceNode memory node3 =
leafDigest(Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000030), data);
NamespaceNode memory node1_2 = nodeDigest(node1, node2);
NamespaceNode memory root = nodeDigest(node1_2, node3);
NamespaceNode memory startingNode = node1_2;

sideNodes[0] = node3;
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
bool isValid = NamespaceMerkleTree.verifyInner(root, proof, startingNode, startingHeight);
assert(isValid);
}

function testVerifyInnerLeafIsRoot() external {
Namespace memory nid = Namespace(0x00, 0x00000000000000000000000000000000000000000000000000000000);
NamespaceNode memory root =
Expand Down Expand Up @@ -292,7 +557,7 @@ contract NamespaceMerkleTreeTest is DSTest {
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
NamespaceNode memory node =
NamespaceNode(nid, nid, 0xc59fa9c4ec515726c2b342544433f844c7b930cf7a5e7abab593332453ceaf70);
uint256 startingHeight = 0;
uint256 startingHeight = 1;
bool isValid = NamespaceMerkleTree.verifyInner(root, proof, node, startingHeight);
assertTrue(!isValid);
}
Expand All @@ -312,7 +577,7 @@ contract NamespaceMerkleTreeTest is DSTest {
NamespaceMerkleProof memory proof = NamespaceMerkleProof(sideNodes, key, numLeaves);
NamespaceNode memory node =
NamespaceNode(nid, nid, 0xc59fa9c4ec515726c2b342544433f844c7b930cf7a5e7abab593332453ceaf70);
uint256 startingHeight = 0;
uint256 startingHeight = 1;
bool isValid = NamespaceMerkleTree.verifyInner(root, proof, node, startingHeight);
assertTrue(!isValid);
}
Expand Down

0 comments on commit 86cbb51

Please sign in to comment.