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

refactor: deobfuscate dragonberry #383

Merged
merged 15 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
62 changes: 40 additions & 22 deletions go/ops.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,38 +20,56 @@ import (
_ "golang.org/x/crypto/ripemd160" //nolint:staticcheck
)

// validate the IAVL Ops
func validateIavlOps(op opType, b int) error {
// validateIavlOps validates the prefix to ensure it begins with
// the height, size, and version of the IAVL tree. Each varint must be a bounded value.
// In addition, the remaining bytes are validated to ensure they correspond to the correct
// length.
func validateIavlOps(op opType, layerNum int) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

b -> layerNum open to other naming suggestions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

layerNum is good to me, maybe we can indicate in godoc that it corresponds to tree depth

r := bytes.NewReader(op.GetPrefix())

values := []int64{}
for i := 0; i < 3; i++ {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to remove the for loop for readability, so each height, size, and version could have a var name rather than an index

varInt, err := binary.ReadVarint(r)
if err != nil {
return err
}
values = append(values, varInt)
height, err := binary.ReadVarint(r)
if err != nil {
return err
}
if int(height) < 0 || int(height) < layerNum {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

previous check: if int(values[0]) < b { moved here in int(height) < layerNum

return fmt.Errorf("IAVL height (%d) must be non-negative and less than the layer number (%d)", height, layerNum)
}

// values must be bounded
if int(varInt) < 0 {
return fmt.Errorf("wrong value in IAVL leaf op")
}
size, err := binary.ReadVarint(r)
if err != nil {
return err
}
if int(values[0]) < b {
return fmt.Errorf("wrong value in IAVL leaf op")

if int(size) < 0 {
return fmt.Errorf("IAVL size must be non-negative")
}

r2 := r.Len()
if b == 0 {
if r2 != 0 {
return fmt.Errorf("invalid op")
version, err := binary.ReadVarint(r)
if err != nil {
return err
}

if int(version) < 0 {
return fmt.Errorf("IAVL version must be non-negative")
}

// grab the length of the remainder of the prefix
remLen := r.Len()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

r2 -> remLen open to other naming suggestions!

if layerNum == 0 {
if remLen != 0 {
return fmt.Errorf("expected remaining prefix to be 0, got: %d", remLen)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
}
} else {
if !(r2^(0xff&0x01) == 0 || r2 == (0xde+int('v'))/10) {
// when the child comes from the left, the suffix if filled in
// prefix: height | size | version | length byte (1 remainder)
//
// when the child comes from the right, the suffix is empty
// prefix: height | size | version | length byte | 32 byte hash | next length byte (34 remainder)
Comment on lines +63 to +67
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @damiannolan for this!

if remLen != 1 && remLen != 34 {
return fmt.Errorf("invalid op")
}
if op.GetHash()^1 != 0 {
return fmt.Errorf("invalid op")
if op.GetHash() != HashOp_SHA256 {
return fmt.Errorf("IAVL hash op must be %v", HashOp_SHA256)
}
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion go/ops_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestInnerOpCheckAgainstSpec(t *testing.T) {
func() {
innerOp.Prefix = []byte{0x01}
},
fmt.Errorf("wrong value in IAVL leaf op"),
fmt.Errorf("IAVL height (-1) must be non-negative and less than the layer number (1)"),
},
{
"failure: inner prefix starts with leaf prefix",
Expand Down
2 changes: 1 addition & 1 deletion go/proof_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestCheckAgainstSpec(t *testing.T) {
if tc.Err == "" && err != nil {
t.Fatalf("Unexpected error: %v", err)
} else if tc.Err != "" && tc.Err != err.Error() {
t.Fatalf("Expected error: %s, got %s", tc.Err, err.Error())
t.Fatalf("Expected error: %s, got: %s", tc.Err, err.Error())
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion testdata/TestCheckAgainstSpecData.json
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,7 @@
"hash": 1
}
},
"Err": "inner, unexpected EOF"
"Err": "inner, IAVL height (0) must be non-negative and less than the layer number (3)"
},
"rejects only inner proof (hash mismatch)": {
"Proof": {
Expand Down
Loading