Skip to content

Commit

Permalink
fix(bits): prevent BitArray.UnmarshalJSON from crashing on 0 bits in …
Browse files Browse the repository at this point in the history
…the JSON (backport #2774) (#2778)

This change fixes a bug in which BitArray.UnmarshalJSON hadn't accounted
for the fact that invoking NewBitArray(<=0) returns nil and hence when
dereferenced would crash with a runtime nil pointer dereference. This
bug was found by my security analysis and fuzzing too.

Author: @odeke-em 

Fixes #2658

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [ ] ~~Updated relevant documentation (`docs/` or `spec/`) and code
comments~~
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #2774 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
  • Loading branch information
mergify[bot] and melekes committed Apr 11, 2024
1 parent 66494ac commit 575b889
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 0 deletions.
@@ -0,0 +1,2 @@
- [`bits`] prevent `BitArray.UnmarshalJSON` from crashing on 0 bits
([\#2774](https://github.com/cometbft/cometbft/pull/2774))
7 changes: 7 additions & 0 deletions libs/bits/bit_array.go
Expand Up @@ -409,6 +409,13 @@ func (bA *BitArray) UnmarshalJSON(bz []byte) error {
// Construct new BitArray and copy over.
numBits := len(bits)
bA2 := NewBitArray(numBits)
if bA2 == nil {
// Treat it as if we encountered the case: b == "null"
bA.Bits = 0
bA.Elems = nil
return nil
}

for i := 0; i < numBits; i++ {
if bits[i] == 'x' {
bA2.SetIndex(i, true)
Expand Down
15 changes: 15 additions & 0 deletions libs/bits/bit_array_test.go
Expand Up @@ -285,3 +285,18 @@ func TestBitArrayProtoBuf(t *testing.T) {
}
}
}

// Tests that UnmarshalJSON doesn't crash when no bits are passed into the JSON.
// See issue https://github.com/cometbft/cometbft/issues/2658
func TestUnmarshalJSONDoesntCrashOnZeroBits(t *testing.T) {
type indexCorpus struct {
BitArray *BitArray `json:"ba"`
Index int `json:"i"`
}

ic := new(indexCorpus)
blob := []byte(`{"BA":""}`)
err := json.Unmarshal(blob, ic)
require.NoError(t, err)
require.Equal(t, ic.BitArray, &BitArray{Bits: 0, Elems: nil})
}

0 comments on commit 575b889

Please sign in to comment.