Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: verifies input range in the computeRoot to avoid panic #186

Merged
merged 42 commits into from
May 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
e49d7b1
updates criteria of an empty proof
staheri14 Apr 21, 2023
5e9091c
replaces slice literal with nID1
staheri14 Apr 21, 2023
b6e3cd3
refactors the code to reject invalid empty proof, adds tests
staheri14 Apr 24, 2023
5c32cbc
corrects IsOfEmptyProof description
staheri14 Apr 24, 2023
84865d7
prefixes min and max with root
staheri14 Apr 24, 2023
c8a8059
fixes linter issues
staheri14 Apr 24, 2023
8a27636
revises some typos
staheri14 Apr 25, 2023
9e3d8de
renames IsOfEmptyProof to IsEmptyProof
staheri14 Apr 25, 2023
20b2205
returns immediately on invalid range
staheri14 Apr 25, 2023
6222d5f
adds unit tests
staheri14 Apr 25, 2023
30cbde4
Merge remote-tracking branch 'origin/master' into verifies-proof-range
staheri14 Apr 25, 2023
9fd31f2
considers start=end as invalid
staheri14 Apr 25, 2023
167629c
defines a utility function for proof range verification
staheri14 Apr 25, 2023
fcd3cd0
returns the error returned by validateRange
staheri14 Apr 26, 2023
d11e9c2
adds proof range check
staheri14 Apr 26, 2023
bb07d07
incorporates further tests covering new range check
staheri14 Apr 26, 2023
d491b52
returns err produced by validateRange
staheri14 Apr 26, 2023
e6cb4dc
Merge branch 'verifies-proof-range' into panics-in-verifyleafhashes
staheri14 Apr 26, 2023
a449d68
implements the necessary logic to handle empty proofs
staheri14 Apr 26, 2023
29007f4
develops tests
staheri14 Apr 26, 2023
8fdaab2
revises the err message
staheri14 Apr 26, 2023
409f0f8
extracts the NID from the leaf
staheri14 Apr 26, 2023
2d504d5
removes an excess line
staheri14 Apr 26, 2023
48010b1
Merge branch 'master' into panics-in-verifyleafhashes
staheri14 Apr 27, 2023
712ed08
revises tests descriptions
staheri14 Apr 27, 2023
e01ce18
declares a variable for nonEmptyProof for readability
staheri14 Apr 27, 2023
fc595aa
removes an excess line
staheri14 Apr 27, 2023
021adc0
replaces manual extraction of min and max NID with proper func calls
staheri14 Apr 27, 2023
951ad75
adds node format validation to the validateSiblingsNamespaceOrder
staheri14 Apr 27, 2023
d132fdc
checks the input range
staheri14 Apr 28, 2023
69d2357
tests the range
staheri14 Apr 28, 2023
2869e6b
deletes an excess line
staheri14 Apr 28, 2023
a35a4e2
considers start=end as valid
staheri14 Apr 28, 2023
81e86bf
implements test for the invalid siblings format
staheri14 Apr 28, 2023
4a4df73
Merge branch 'resolves-panic-validateSiblings' into compute-root-rang…
staheri14 Apr 28, 2023
f70d2d7
resolves linter issues
staheri14 Apr 28, 2023
41dfd1e
Merge branch 'resolves-panic-validateSiblings' into compute-root-rang…
staheri14 Apr 28, 2023
53597b9
Merge branch 'master' into resolves-panic-validateSiblings
staheri14 May 1, 2023
c3c8017
Merge branch 'resolves-panic-validateSiblings' into compute-root-rang…
staheri14 May 1, 2023
ef334fe
Merge branch 'master' into compute-root-range-check
staheri14 May 1, 2023
e446d76
edits comments and tests descriptions
staheri14 May 1, 2023
eecd425
tests for start less than zero and end larger than the tree size
staheri14 May 2, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions nmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,11 @@ func (n *NamespacedMerkleTree) MaxNamespace() (namespace.ID, error) {
// encompasses the leaves within the range of [start, end).
// Any errors returned by this method are irrecoverable and indicate an illegal state of the tree (n).
func (n *NamespacedMerkleTree) computeRoot(start, end int) ([]byte, error) {
// in computeRoot, start may be equal to end which indicates an empty tree hence empty root.
// Due to this, we need to perform custom range check instead of using validateRange() in which start=end is considered invalid.
if start < 0 || start > end || end > len(n.leaves) {
return nil, fmt.Errorf("failed to compute root [%d, %d): %w", start, end, ErrInvalidRange)
}
switch end - start {
case 0:
rootHash := n.treeHasher.EmptyRoot()
Expand Down
25 changes: 17 additions & 8 deletions nmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1019,30 +1019,39 @@ func Test_Root_Error(t *testing.T) {

// Test_computeRoot_Error tests that the computeRoot method returns an error when the underlying tree is in an invalid state, such as when the leaves are not ordered by namespace ID or when a leaf is corrupt.
func Test_computeRoot_Error(t *testing.T) {
nIDSize := 2
nIDList := []byte{1, 2, 3, 4, 5, 6, 7, 8}

// create an NMT with 8 sequentially namespaced leaves, numbered from 1 to 8.
treeWithCorruptLeafHash := exampleNMT(2, 1, 2, 3, 4, 5, 6, 7, 8)
treeWithCorruptLeafHash := exampleNMT(nIDSize, nIDList...)
// corrupt a leaf hash
treeWithCorruptLeafHash.leafHashes[4] = treeWithCorruptLeafHash.leafHashes[4][:treeWithCorruptLeafHash.NamespaceSize()-1]

// create an NMT with 8 sequentially namespaced leaves, numbered from 1 to 8.
treeWithUnorderedLeaves := exampleNMT(2, 1, 2, 3, 4, 5, 6, 7, 8)
treeWithUnorderedLeaves := exampleNMT(nIDSize, nIDList...)
// swap the positions of the 4th and 5th leaves
swap(treeWithUnorderedLeaves.leaves, 4, 5)
swap(treeWithUnorderedLeaves.leafHashes, 4, 5)

// create an NMT with 8 sequentially namespaced leaves, numbered from 1 to 8.
validTree := exampleNMT(nIDSize, nIDList...)

tests := []struct {
name string
tree *NamespacedMerkleTree
start, end int
wantErr bool
errType error
}{
{"corrupt leaf hash: the entire tree", treeWithCorruptLeafHash, 0, 7, true, ErrInvalidNodeLen},
{"corrupt leaf: from the corrupt node until the end of the tree", treeWithCorruptLeafHash, 4, 7, true, ErrInvalidNodeLen},
{"corrupt leaf: the corrupt node and the node to its left", treeWithCorruptLeafHash, 3, 5, true, ErrInvalidNodeLen},
{"unordered leaves: the entire tree", treeWithUnorderedLeaves, 0, 7, true, ErrUnorderedSiblings},
{"unordered leaves: the unordered portion", treeWithUnorderedLeaves, 4, 6, true, ErrUnorderedSiblings},
{"unordered leaves: a portion of the tree containing the unordered leaves", treeWithUnorderedLeaves, 3, 7, true, ErrUnorderedSiblings},
{"invalid tree with corrupt leaf hash. Query: the entire tree", treeWithCorruptLeafHash, 0, 7, true, ErrInvalidNodeLen},
{"invalid tree with corrupt leaf. Query: from the corrupt node until the end of the tree", treeWithCorruptLeafHash, 4, 7, true, ErrInvalidNodeLen},
{"invalid tree with corrupt leaf. Query: the corrupt node and the node to its left", treeWithCorruptLeafHash, 3, 5, true, ErrInvalidNodeLen},
{"invalid tree with unordered leaves. Query: the entire tree", treeWithUnorderedLeaves, 0, 7, true, ErrUnorderedSiblings},
{"invalid tree with unordered leaves. Query: the unordered portion", treeWithUnorderedLeaves, 4, 6, true, ErrUnorderedSiblings},
{"invalid tree with unordered leaves. Query: a portion of the tree containing the unordered leaves", treeWithUnorderedLeaves, 3, 7, true, ErrUnorderedSiblings},
{"valid tree. Query: start < 0", validTree, -1, 1, true, ErrInvalidRange},
{"valid tree. Query: start > end", validTree, 3, 1, true, ErrInvalidRange},
{"valid tree. Query: end > total number of leaves", validTree, 3, len(validTree.leaves) + 1, true, ErrInvalidRange},
}
rootulp marked this conversation as resolved.
Show resolved Hide resolved
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down