Skip to content

Commit 93a99b7

Browse files
marcello33mattac21
andauthored
sec patch (#32)
Co-authored-by: Matt Acciai <matt@cosmoslabs.io>
1 parent 076595d commit 93a99b7

5 files changed

Lines changed: 156 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 46 additions & 0 deletions
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*

consensus/reactor.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1684,6 +1684,9 @@ func (m *NewValidBlockMessage) ValidateBasic() error {
16841684
if err := m.BlockPartSetHeader.ValidateBasic(); err != nil {
16851685
return fmt.Errorf("wrong BlockPartSetHeader: %v", err)
16861686
}
1687+
if err := m.BlockParts.ValidateBasic(); err != nil {
1688+
return fmt.Errorf("validating BlockParts: %w", err)
1689+
}
16871690
if m.BlockParts.Size() == 0 {
16881691
return errors.New("empty blockParts")
16891692
}
@@ -1738,6 +1741,9 @@ func (m *ProposalPOLMessage) ValidateBasic() error {
17381741
if m.ProposalPOLRound < 0 {
17391742
return errors.New("negative ProposalPOLRound")
17401743
}
1744+
if err := m.ProposalPOL.ValidateBasic(); err != nil {
1745+
return fmt.Errorf("validating ProposalPOL: %w", err)
1746+
}
17411747
if m.ProposalPOL.Size() == 0 {
17421748
return errors.New("empty ProposalPOL bit array")
17431749
}
@@ -1912,6 +1918,9 @@ func (m *VoteSetBitsMessage) ValidateBasic() error {
19121918
if err := m.BlockID.ValidateBasic(); err != nil {
19131919
return fmt.Errorf("wrong BlockID: %v", err)
19141920
}
1921+
if err := m.Votes.ValidateBasic(); err != nil {
1922+
return fmt.Errorf("validating Votes: %w", err)
1923+
}
19151924
// NOTE: Votes.Size() can be zero if the node does not have any
19161925
if m.Votes.Size() > types.MaxVotesCount {
19171926
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
@@ -912,6 +912,14 @@ func TestNewValidBlockMessageValidateBasic(t *testing.T) {
912912
func(msg *NewValidBlockMessage) { msg.BlockParts = bits.NewBitArray(int(types.MaxBlockPartsCount) + 1) },
913913
"blockParts bit array size 1601 not equal to BlockPartSetHeader.Total 1",
914914
},
915+
{
916+
func(msg *NewValidBlockMessage) { msg.BlockParts.Elems = nil },
917+
"mismatch between specified number of bits 1, and number of elements 0, expected 1 elements",
918+
},
919+
{
920+
func(msg *NewValidBlockMessage) { msg.BlockParts.Bits = 500 },
921+
"mismatch between specified number of bits 500, and number of elements 1, expected 8 elements",
922+
},
915923
}
916924

917925
for i, tc := range testCases {
@@ -948,6 +956,14 @@ func TestProposalPOLMessageValidateBasic(t *testing.T) {
948956
func(msg *ProposalPOLMessage) { msg.ProposalPOL = bits.NewBitArray(types.MaxVotesCount + 1) },
949957
"proposalPOL bit array is too big: 10001, max: 10000",
950958
},
959+
{
960+
func(msg *ProposalPOLMessage) { msg.ProposalPOL.Elems = nil },
961+
"mismatch between specified number of bits 1, and number of elements 0, expected 1 elements",
962+
},
963+
{
964+
func(msg *ProposalPOLMessage) { msg.ProposalPOL.Bits = 500 },
965+
"mismatch between specified number of bits 500, and number of elements 1, expected 8 elements",
966+
},
951967
}
952968

953969
for i, tc := range testCases {
@@ -1139,6 +1155,14 @@ func TestVoteSetBitsMessageValidateBasic(t *testing.T) {
11391155
func(msg *VoteSetBitsMessage) { msg.Votes = bits.NewBitArray(types.MaxVotesCount + 1) },
11401156
"votes bit array is too big: 10001, max: 10000",
11411157
},
1158+
{
1159+
func(msg *VoteSetBitsMessage) { msg.Votes.Elems = nil },
1160+
"mismatch between specified number of bits 1, and number of elements 0, expected 1 elements",
1161+
},
1162+
{
1163+
func(msg *VoteSetBitsMessage) { msg.Votes.Bits = 500 },
1164+
"mismatch between specified number of bits 500, and number of elements 1, expected 8 elements",
1165+
},
11421166
}
11431167

11441168
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)
@@ -90,7 +90,7 @@ func (bA *BitArray) SetIndex(i int, v bool) bool {
9090
}
9191

9292
func (bA *BitArray) setIndex(i int, v bool) bool {
93-
if i >= bA.Bits {
93+
if i >= bA.Bits || i/64 >= len(bA.Elems) {
9494
return false
9595
}
9696
if v {
@@ -121,7 +121,7 @@ func (bA *BitArray) copy() *BitArray {
121121
}
122122

123123
func (bA *BitArray) copyBits(bits int) *BitArray {
124-
c := make([]uint64, (bits+63)/64)
124+
c := make([]uint64, numElements(bits))
125125
copy(c, bA.Elems)
126126
return &BitArray{
127127
Bits: bits,
@@ -282,6 +282,11 @@ func (bA *BitArray) PickRandom() (int, bool) {
282282
}
283283

284284
func (bA *BitArray) getNumTrueIndices() int {
285+
if bA.Size() == 0 || len(bA.Elems) == 0 || len(bA.Elems) != numElements(bA.Size()) {
286+
// size and elements must be valid to do this calc
287+
return 0
288+
}
289+
285290
count := 0
286291
numElems := len(bA.Elems)
287292
// handle all elements except the last one
@@ -495,3 +500,22 @@ func (bA *BitArray) FromProto(protoBitArray *cmtprotobits.BitArray) {
495500
bA.Elems = protoBitArray.Elems
496501
}
497502
}
503+
504+
// ValidateBasic validates a BitArray. Note that a nil BitArray and BitArray of
505+
// size 0 bits is valid. However the number of Bits and Elems be valid based on
506+
// each other.
507+
func (bA *BitArray) ValidateBasic() error {
508+
if bA == nil {
509+
return nil
510+
}
511+
512+
expectedElems := numElements(bA.Size())
513+
if expectedElems != len(bA.Elems) {
514+
return fmt.Errorf("mismatch between specified number of bits %d, and number of elements %d, expected %d elements", bA.Size(), len(bA.Elems), expectedElems)
515+
}
516+
return nil
517+
}
518+
519+
func numElements(bits int) int {
520+
return (bits + 63) / 64
521+
}

libs/bits/bit_array_test.go

Lines changed: 49 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) {
@@ -371,6 +397,28 @@ func TestBitArrayProtoBuf(t *testing.T) {
371397
}
372398
}
373399

400+
func TestBitArrayValidateBasic(t *testing.T) {
401+
testCases := []struct {
402+
name string
403+
bA1 *BitArray
404+
expPass bool
405+
}{
406+
{"valid empty", &BitArray{}, true},
407+
{"valid explicit 0 bits nil elements", &BitArray{Bits: 0, Elems: nil}, true},
408+
{"valid explicit 0 bits 0 len elements", &BitArray{Bits: 0, Elems: make([]uint64, 0)}, true},
409+
{"valid nil", nil, true},
410+
{"valid with elements", NewBitArray(10), true},
411+
{"more elements than bits specifies", &BitArray{Bits: 0, Elems: make([]uint64, 5)}, false},
412+
{"less elements than bits specifies", &BitArray{Bits: 200, Elems: make([]uint64, 1)}, false},
413+
}
414+
for _, tc := range testCases {
415+
t.Run(tc.name, func(t *testing.T) {
416+
err := tc.bA1.ValidateBasic()
417+
require.Equal(t, err == nil, tc.expPass)
418+
})
419+
}
420+
}
421+
374422
// Tests that UnmarshalJSON doesn't crash when no bits are passed into the JSON.
375423
// See issue https://github.com/cometbft/cometbft/issues/2658
376424
func TestUnmarshalJSONDoesntCrashOnZeroBits(t *testing.T) {

0 commit comments

Comments
 (0)