From a8f5fcd7422ac7a38ab9dc54d2c054fa7fb1b36f Mon Sep 17 00:00:00 2001 From: jholdstock Date: Fri, 7 Aug 2026 12:03:13 +0800 Subject: [PATCH] merkle: Validate web request inputs. Previously merkleBranch.extract would trust the fields set in merkleBranch implicitly, which could lead a panic when values are set from a web requests without prior validation. --- merkle/merkle.go | 16 ++++++++++-- merkle/merkle_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/merkle/merkle.go b/merkle/merkle.go index 4634a06..86bfe22 100644 --- a/merkle/merkle.go +++ b/merkle/merkle.go @@ -1,5 +1,5 @@ // Copyright (c) 2013-2016 The btcsuite developers -// Copyright (c) 2015-2020 The Decred developers +// Copyright (c) 2015-2026 The Decred developers // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. @@ -15,7 +15,11 @@ import ( "sort" ) -var ErrEmpty = errors.New("empty merkle branch") +var ( + ErrEmpty = errors.New("empty merkle branch") + errNotEnoughFlagBits = errors.New("not enough flag bits") + errNotEnoughHashes = errors.New("not enough hashes") +) type sortableSlice []*[sha256.Size]byte @@ -277,9 +281,17 @@ type merkleBranch struct { // extract recurses over the merkleBranch and returns the merkle root. func (m *merkleBranch) extract(height, pos uint32) (*[sha256.Size]byte, error) { + // merkleBranch fields may be decoded straight from a web request, so the + // bits and hashes fields must be checked before use. + if m.bitsUsed >= uint32(len(m.bits)) { + return nil, errNotEnoughFlagBits + } parentOfMatch := m.bits[m.bitsUsed] m.bitsUsed++ if height == 0 || parentOfMatch == 0 { + if m.hashUsed >= uint32(len(m.inHashes)) { + return nil, errNotEnoughHashes + } hash := m.inHashes[m.hashUsed] m.hashUsed++ if height == 0 && parentOfMatch == 1 { diff --git a/merkle/merkle_test.go b/merkle/merkle_test.go index 1536910..2719d35 100644 --- a/merkle/merkle_test.go +++ b/merkle/merkle_test.go @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2020 The Decred developers +// Copyright (c) 2017-2026 The Decred developers // Use of this source code is governed by an ISC // license that can be found in the LICENSE file. @@ -9,6 +9,8 @@ import ( "crypto/sha256" "encoding/binary" "encoding/hex" + "errors" + "math" "testing" ) @@ -200,3 +202,58 @@ func TestAuthPathEmpty(t *testing.T) { t.Fatalf("Should have gotten nil") } } + +// TestExtractInvalid ensures that attempting to extract from a merkle branch +// with an invalid combination of fields returns an error. +func TestExtractInvalid(t *testing.T) { + hash := [sha256.Size]byte{} + binary.LittleEndian.PutUint64(hash[:], 1) + + tests := []struct { + name string + numLeaves uint32 + hashes [][sha256.Size]byte + bits []byte + wantErr error + }{{ + name: "flag bits run out", + numLeaves: 1000, + hashes: [][sha256.Size]byte{hash}, + bits: []byte{0xff}, + wantErr: errNotEnoughFlagBits, + }, { + name: "no flag bits", + numLeaves: 4, + hashes: [][sha256.Size]byte{hash}, + bits: []byte{}, + wantErr: errNotEnoughFlagBits, + }, { + name: "hashes run out", + numLeaves: 4, + hashes: [][sha256.Size]byte{hash}, + bits: []byte{0xff, 0xff}, + wantErr: errNotEnoughHashes, + }, { + name: "no hashes", + numLeaves: 4, + hashes: [][sha256.Size]byte{}, + bits: []byte{0xff}, + wantErr: errNotEnoughHashes, + }} + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + m := &merkleBranch{ + bits: bytes2bits(test.bits), + inHashes: test.hashes, + numLeaves: test.numLeaves, + } + + height := uint32(math.Ceil(math.Log2(float64(test.numLeaves)))) + _, err := m.extract(height, 0) + if !errors.Is(err, test.wantErr) { + t.Fatalf("got error %q, want %q", err, test.wantErr) + } + }) + } +}