Skip to content

Commit f38454d

Browse files
rach-idrootulpmattac21
authored
fix: Invalid BitArray handling on main (#2574)
Closes: #2573 --------- Co-authored-by: Rootul Patel <[email protected]> Co-authored-by: Matt Acciai <[email protected]>
1 parent cfbf301 commit f38454d

File tree

5 files changed

+196
-6
lines changed

5 files changed

+196
-6
lines changed

‎CHANGELOG.md‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,51 @@
11
# CHANGELOG
22

3+
## UNRELEASED
4+
5+
### DEPENDENCIES
6+
7+
### BUG FIXES
8+
9+
### IMPROVEMENTS
10+
11+
### FEATURES
12+
13+
### BUG-FIXES
14+
15+
### STATE-BREAKING
16+
17+
### API-BREAKING
18+
19+
## v0.38.19
20+
21+
*October 14, 2025*
22+
23+
This release fixes two security issues, including ([ASA-2025-003](https://github.com/cometbft/cometbft/security/advisories/GHSA-hrhf-2vcr-ghch)).
24+
Users are encouraged to upgrade as soon as possible.
25+
26+
Additionally included is a bug fix to properly prune extended commits (with
27+
vote extensions).
28+
29+
### BUG-FIXES
30+
31+
- `[consensus]` Reject oversized proposals
32+
([\#5324](https://github.com/cometbft/cometbft/pull/5324))
33+
- `[store]` Prune extended commits properly
34+
([5275](https://github.com/cometbft/cometbft/issues/5275))
35+
- `[bits]` Validate BitArray mismatched Bits and Elems length
36+
([ASA-2025-003](https://github.com/cometbft/cometbft/security/advisories/GHSA-hrhf-2vcr-ghch))
37+
38+
## v0.38.18
39+
40+
*July 3, 2025*
41+
42+
Adds precommit metrics and reindex CLI command.
43+
44+
### IMPROVEMENTS
45+
46+
- Adds metrics that emit precommit data; precommit quorum delay from proposal, and precommit vote count and stake weight within timeout commit period.
47+
([\#5251](https://github.com/cometbft/cometbft/issues/5251))
48+
349
## v0.38.17
450

551
*February 3, 2025*
@@ -881,4 +927,3 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/cosmos).
881927
## Previous changes
882928

883929
For changes released before the creation of CometBFT, please refer to the Tendermint Core [CHANGELOG.md](https://github.com/tendermint/tendermint/blob/a9feb1c023e172b542c972605311af83b777855b/CHANGELOG.md).
884-

‎consensus/reactor.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1817,6 +1817,9 @@ func (m *NewValidBlockMessage) ValidateBasic() error {
18171817
if err := m.BlockPartSetHeader.ValidateBasic(); err != nil {
18181818
return fmt.Errorf("wrong BlockPartSetHeader: %v", err)
18191819
}
1820+
if err := m.BlockParts.ValidateBasic(); err != nil {
1821+
return fmt.Errorf("validating BlockParts: %w", err)
1822+
}
18201823
if m.BlockParts.Size() == 0 {
18211824
return errors.New("empty blockParts")
18221825
}
@@ -1871,6 +1874,9 @@ func (m *ProposalPOLMessage) ValidateBasic() error {
18711874
if m.ProposalPOLRound < 0 {
18721875
return errors.New("negative ProposalPOLRound")
18731876
}
1877+
if err := m.ProposalPOL.ValidateBasic(); err != nil {
1878+
return fmt.Errorf("validating ProposalPOL: %w", err)
1879+
}
18741880
if m.ProposalPOL.Size() == 0 {
18751881
return errors.New("empty ProposalPOL bit array")
18761882
}
@@ -2016,6 +2022,9 @@ func (m *VoteSetBitsMessage) ValidateBasic() error {
20162022
if err := m.BlockID.ValidateBasic(); err != nil {
20172023
return fmt.Errorf("wrong BlockID: %v", err)
20182024
}
2025+
if err := m.Votes.ValidateBasic(); err != nil {
2026+
return fmt.Errorf("validating Votes: %w", err)
2027+
}
20192028
// NOTE: Votes.Size() can be zero if the node does not have any
20202029
if m.Votes.Size() > types.MaxVotesCount {
20212030
return fmt.Errorf("votes bit array is too big: %d, max: %d", m.Votes.Size(), types.MaxVotesCount)

‎consensus/reactor_test.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,14 @@ func TestNewValidBlockMessageValidateBasic(t *testing.T) {
907907
func(msg *NewValidBlockMessage) { msg.BlockParts = bits.NewBitArray(int(types.MaxBlockPartsCount) + 1) },
908908
fmt.Sprintf("blockParts bit array size %d not equal to BlockPartSetHeader.Total 1", types.MaxBlockPartsCount+1),
909909
},
910+
{
911+
func(msg *NewValidBlockMessage) { msg.BlockParts.Elems = nil },
912+
"mismatch between specified number of bits 1, and number of elements 0, expected 1 elements",
913+
},
914+
{
915+
func(msg *NewValidBlockMessage) { msg.BlockParts.Bits = 500 },
916+
"mismatch between specified number of bits 500, and number of elements 1, expected 8 elements",
917+
},
910918
}
911919

912920
for i, tc := range testCases {
@@ -943,6 +951,14 @@ func TestProposalPOLMessageValidateBasic(t *testing.T) {
943951
func(msg *ProposalPOLMessage) { msg.ProposalPOL = bits.NewBitArray(types.MaxVotesCount + 1) },
944952
"proposalPOL bit array is too big: 10001, max: 10000",
945953
},
954+
{
955+
func(msg *ProposalPOLMessage) { msg.ProposalPOL.Elems = nil },
956+
"mismatch between specified number of bits 1, and number of elements 0, expected 1 elements",
957+
},
958+
{
959+
func(msg *ProposalPOLMessage) { msg.ProposalPOL.Bits = 500 },
960+
"mismatch between specified number of bits 500, and number of elements 1, expected 8 elements",
961+
},
946962
}
947963

948964
for i, tc := range testCases {
@@ -1099,6 +1115,14 @@ func TestVoteSetBitsMessageValidateBasic(t *testing.T) {
10991115
func(msg *VoteSetBitsMessage) { msg.Votes = bits.NewBitArray(types.MaxVotesCount + 1) },
11001116
"votes bit array is too big: 10001, max: 10000",
11011117
},
1118+
{
1119+
func(msg *VoteSetBitsMessage) { msg.Votes.Elems = nil },
1120+
"mismatch between specified number of bits 1, and number of elements 0, expected 1 elements",
1121+
},
1122+
{
1123+
func(msg *VoteSetBitsMessage) { msg.Votes.Bits = 500 },
1124+
"mismatch between specified number of bits 500, and number of elements 1, expected 8 elements",
1125+
},
11021126
}
11031127

11041128
for i, tc := range testCases {

‎libs/bits/bit_array.go‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func NewBitArray(bits int) *BitArray {
2828
}
2929
return &BitArray{
3030
Bits: bits,
31-
Elems: make([]uint64, (bits+63)/64),
31+
Elems: make([]uint64, numElements(bits)),
3232
}
3333
}
3434

@@ -41,7 +41,7 @@ func NewBitArrayFromFn(bits int, fn func(int) bool) *BitArray {
4141
}
4242
bA := &BitArray{
4343
Bits: bits,
44-
Elems: make([]uint64, (bits+63)/64),
44+
Elems: make([]uint64, numElements(bits)),
4545
}
4646
for i := 0; i < bits; i++ {
4747
v := fn(i)
@@ -101,7 +101,7 @@ func (bA *BitArray) SetIndex(i int, v bool) bool {
101101
}
102102

103103
func (bA *BitArray) setIndex(i int, v bool) bool {
104-
if i >= bA.Bits {
104+
if i >= bA.Bits || i/64 >= len(bA.Elems) {
105105
return false
106106
}
107107
if v {
@@ -159,7 +159,7 @@ func (bA *BitArray) copy() *BitArray {
159159
}
160160

161161
func (bA *BitArray) copyBits(bits int) *BitArray {
162-
c := make([]uint64, (bits+63)/64)
162+
c := make([]uint64, numElements(bits))
163163
copy(c, bA.Elems)
164164
return &BitArray{
165165
Bits: bits,
@@ -354,6 +354,11 @@ func (bA *BitArray) GetTrueIndices() []int {
354354
}
355355

356356
func (bA *BitArray) getNumTrueIndices() int {
357+
if bA == nil || bA.Bits == 0 || len(bA.Elems) == 0 || len(bA.Elems) != numElements(bA.Bits) {
358+
// size and elements must be valid to do this calc
359+
return 0
360+
}
361+
357362
count := 0
358363
numElems := len(bA.Elems)
359364
// handle all elements except the last one
@@ -567,3 +572,22 @@ func (bA *BitArray) FromProto(protoBitArray *cmtprotobits.BitArray) {
567572
bA.Elems = protoBitArray.Elems
568573
}
569574
}
575+
576+
// ValidateBasic validates a BitArray. Note that a nil BitArray and BitArray of
577+
// size 0 bits is valid. However the number of Bits and Elems be valid based on
578+
// each other.
579+
func (bA *BitArray) ValidateBasic() error {
580+
if bA == nil {
581+
return nil
582+
}
583+
584+
expectedElems := numElements(bA.Size())
585+
if expectedElems != len(bA.Elems) {
586+
return fmt.Errorf("mismatch between specified number of bits %d, and number of elements %d, expected %d elements", bA.Size(), len(bA.Elems), expectedElems)
587+
}
588+
return nil
589+
}
590+
591+
func numElements(bits int) int {
592+
return (bits + 63) / 64
593+
}

‎libs/bits/bit_array_test.go‎

Lines changed: 89 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,28 @@ func TestGetNumTrueIndices(t *testing.T) {
173173
}
174174
}
175175

176+
func TestGetNumTrueIndicesInvalidStates(t *testing.T) {
177+
testCases := []struct {
178+
name string
179+
bA1 *BitArray
180+
exp int
181+
}{
182+
{"empty", &BitArray{}, 0},
183+
{"explicit 0 bits nil elements", &BitArray{Bits: 0, Elems: nil}, 0},
184+
{"explicit 0 bits 0 len elements", &BitArray{Bits: 0, Elems: make([]uint64, 0)}, 0},
185+
{"nil", nil, 0},
186+
{"with elements", NewBitArray(10), 0},
187+
{"more elements than bits specifies", &BitArray{Bits: 0, Elems: make([]uint64, 5)}, 0},
188+
{"less elements than bits specifies", &BitArray{Bits: 200, Elems: make([]uint64, 1)}, 0},
189+
}
190+
for _, tc := range testCases {
191+
t.Run(tc.name, func(t *testing.T) {
192+
n := tc.bA1.getNumTrueIndices()
193+
require.Equal(t, n, tc.exp)
194+
})
195+
}
196+
}
197+
176198
func TestGetNthTrueIndex(t *testing.T) {
177199
type testcase struct {
178200
Input string
@@ -226,7 +248,7 @@ func TestGetNthTrueIndex(t *testing.T) {
226248
}
227249
}
228250

229-
func TestBytes(_ *testing.T) {
251+
func TestBytes(t *testing.T) {
230252
bA := NewBitArray(4)
231253
bA.SetIndex(0, true)
232254
check := func(bA *BitArray, bz []byte) {
@@ -253,6 +275,10 @@ func TestBytes(_ *testing.T) {
253275
check(bA, []byte{0x80, 0x01})
254276
bA.SetIndex(9, true)
255277
check(bA, []byte{0x80, 0x03})
278+
279+
bA = NewBitArray(4)
280+
bA.Elems = nil
281+
require.False(t, bA.SetIndex(1, true))
256282
}
257283

258284
func TestEmptyFull(t *testing.T) {
@@ -477,6 +503,28 @@ func TestAddBitArray(t *testing.T) {
477503
}
478504
}
479505

506+
func TestBitArrayValidateBasic(t *testing.T) {
507+
testCases := []struct {
508+
name string
509+
bA1 *BitArray
510+
expPass bool
511+
}{
512+
{"valid empty", &BitArray{}, true},
513+
{"valid explicit 0 bits nil elements", &BitArray{Bits: 0, Elems: nil}, true},
514+
{"valid explicit 0 bits 0 len elements", &BitArray{Bits: 0, Elems: make([]uint64, 0)}, true},
515+
{"valid nil", nil, true},
516+
{"valid with elements", NewBitArray(10), true},
517+
{"more elements than bits specifies", &BitArray{Bits: 0, Elems: make([]uint64, 5)}, false},
518+
{"less elements than bits specifies", &BitArray{Bits: 200, Elems: make([]uint64, 1)}, false},
519+
}
520+
for _, tc := range testCases {
521+
t.Run(tc.name, func(t *testing.T) {
522+
err := tc.bA1.ValidateBasic()
523+
require.Equal(t, err == nil, tc.expPass)
524+
})
525+
}
526+
}
527+
480528
// Tests that UnmarshalJSON doesn't crash when no bits are passed into the JSON.
481529
// See issue https://github.com/cometbft/cometbft/issues/2658
482530
func TestUnmarshalJSONDoesntCrashOnZeroBits(t *testing.T) {
@@ -541,3 +589,43 @@ func TestBitArray_Fill(t *testing.T) {
541589
require.True(t, bA.IsFull())
542590
})
543591
}
592+
593+
func TestBitArraySize(t *testing.T) {
594+
t.Run("Size returns 0 for nil BitArray", func(t *testing.T) {
595+
var nilArray *BitArray
596+
require.Equal(t, 0, nilArray.Size())
597+
})
598+
599+
t.Run("Size returns the correct size for the BitArray", func(t *testing.T) {
600+
sizes := []int{1, 10, 64, 65, 100, 1000}
601+
for _, size := range sizes {
602+
bitArray := NewBitArray(size)
603+
require.Equal(t, size, bitArray.Size())
604+
}
605+
})
606+
607+
t.Run("Size returns the correct size when called concurrently", func(t *testing.T) {
608+
const numGoroutines = 100
609+
const numIterations = 100
610+
611+
want := 500
612+
bitArray := NewBitArray(want)
613+
614+
results := make(chan int, numGoroutines*numIterations)
615+
616+
// Launch goroutines that only call Size() concurrently
617+
for i := 0; i < numGoroutines; i++ {
618+
go func() {
619+
for j := 0; j < numIterations; j++ {
620+
size := bitArray.Size()
621+
results <- size
622+
}
623+
}()
624+
}
625+
626+
for i := 0; i < numGoroutines*numIterations; i++ {
627+
got := <-results
628+
require.Equal(t, want, got)
629+
}
630+
})
631+
}

0 commit comments

Comments
 (0)