internal/bits: BitArray.UnmarshalJSON can crash if the number of bits in the JSON is 0 #2658
Description
Bug Report
Found by fuzzing and analysis that if I run this simple repro
import (
"encoding/json"
"testing"
)
type indexCorpus struct {
BitArray *BitArray `json:"ba"`
Index int `json:"i"`
}
func TestReproNilDereference(t *testing.T) {
ic := new(indexCorpus)
blob := []byte(`{"BA":""}`)
_ = json.Unmarshal(blob, ic)
}the Unmarshalling panics with
$ go test -run=Repro -v
=== RUN TestReproNilDereference
--- FAIL: TestReproNilDereference (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x93234ae]
goroutine 18 [running]:
testing.tRunner.func1.2({0x93f1960, 0x969b820})
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1631 +0x24a
testing.tRunner.func1()
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1634 +0x377
panic({0x93f1960?, 0x969b820?})
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/runtime/panic.go:759 +0x132
github.com/cometbft/cometbft/internal/bits.(*BitArray).UnmarshalJSON(0xc0001d0390, {0xc0000af0e6?, 0x0?, 0x0?})
/Users/emmanuelodeke/go/src/github.com/cometbft/cometbft/internal/bits/bit_array.go:420 +0x20e
encoding/json.(*decodeState).literalStore(0xc0001b6d80, {0xc0000af0e6, 0x2, 0x3}, {0x944a780?, 0xc000091a00?, 0x1?}, 0x0)
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/encoding/json/decode.go:854 +0x221e
encoding/json.(*decodeState).value(0xc0001b6d80, {0x944a780?, 0xc000091a00?, 0x2?})
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/encoding/json/decode.go:388 +0x115
encoding/json.(*decodeState).object(0xc0001b6d80, {0x93cf3a0?, 0xc000091a00?, 0x50270cc8?})
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/encoding/json/decode.go:755 +0xd11
encoding/json.(*decodeState).value(0xc0001b6d80, {0x93cf3a0?, 0xc000091a00?, 0x96af200?})
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/encoding/json/decode.go:374 +0x3e
encoding/json.(*decodeState).unmarshal(0xc0001b6d80, {0x93cf3a0?, 0xc000091a00?})
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/encoding/json/decode.go:181 +0x11e
encoding/json.Unmarshal({0xc0000af0e0, 0x9, 0x9}, {0x93cf3a0, 0xc000091a00})
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/encoding/json/decode.go:108 +0xf9
github.com/cometbft/cometbft/internal/bits.TestReproNilDereference(0xc0000a31e0)
/Users/emmanuelodeke/go/src/github.com/cometbft/cometbft/internal/bits/fuzz_test.go:54 +0x65
testing.tRunner(0xc0000a31e0, 0x9458788)
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 1
/Users/emmanuelodeke/go/src/go.googlesource.com/go/src/testing/testing.go:1742 +0x390
exit status 2
FAIL github.com/cometbft/cometbft/internal/bits 0.538sRemedy
The reason for the panic is that due to
cometbft/internal/bits/bit_array.go
Lines 410 to 411 in a24eaeb
the result from NewBitArray is not checked yet it returns a nil if
len(bits) == 0
We should defensively check and ensure that we don't nil dereference values, like this
diff --git a/internal/bits/bit_array.go b/internal/bits/bit_array.go
index 4e00b3f9e..3554a2846 100644
--- a/internal/bits/bit_array.go
+++ b/internal/bits/bit_array.go
@@ -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 encounter 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)