CVE-2023-23626: Improper Validation of Specified Quantity in Input (CWE-1284)
value: go-bitfield is a simple bitfield package for the go language aiming to be more performant that the standard library. When feeding untrusted user input into the size parameter of `NewBitfield` and `FromBytes` functions, an attacker can trigger `panic`s. This happen when the `size` is a not a multiple of `8` or is negative. There were already a note in the `NewBitfield` documentation, however known users of this package are subject to this issue. Users are advised to upgrade. Users unable to upgrade should ensure that `size` is a multiple of 8 before calling `NewBitfield` or `FromBytes`.

CWE description: The product receives input that is expected to specify a quantity (such as size or length), but it does not validate or incorrectly validates that the quantity has the required properties.
CVSS v3 base score: 7.5
Published: 2023-02-09T21:15Z
Repository: https://github.com/ipfs/go-bitfield
Language: Go
Files changed: bitfield.go, bitfield_test.go
Commit date: 2023-01-26 05:06:57 +0100
Commit message: refactor: return errors instead of panics

It is clearer and simpler for consumers if we use the type system instead of relying on them for extra checks.

Fixes https://github.com/ipfs/go-bitfield/security/advisories/GHSA-2h6c-j3gf-xp9r

Diff: diff --git a/bitfield.go b/bitfield.go
index d6f12b0..2f77646 100644
--- a/bitfield.go
+++ b/bitfield.go
@@ -1,32 +1,37 @@
 package bitfield
 
 // NOTE: Don't bother replacing the divisions/modulo with shifts/ands, go is smart.
 
 import (
+	"fmt"
 	"math/bits"
 )
 
 // NewBitfield creates a new fixed-sized Bitfield (allocated up-front).
-//
-// Panics if size is not a multiple of 8.
-func NewBitfield(size int) Bitfield {
+func NewBitfield(size int) (Bitfield, error) {
+	if size < 0 {
+		return nil, fmt.Errorf("bitfield size must be positive; got %d", size)
+	}
 	if size%8 != 0 {
-		panic("Bitfield size must be a multiple of 8")
+		return nil, fmt.Errorf("bitfield size must be a multiple of 8; got %d", size)
 	}
-	return make([]byte, size/8)
+	return make([]byte, size/8), nil
 }
 
 // FromBytes constructs a new bitfield from a serialized bitfield.
-func FromBytes(size int, bits []byte) Bitfield {
-	bf := NewBitfield(size)
+func FromBytes(size int, bits []byte) (Bitfield, error) {
+	bf, err := NewBitfield(size)
+	if err != nil {
+		return nil, err
+	}
 	start := len(bf) - len(bits)
 	if start < 0 {
-		panic("bitfield too small")
+		return nil, fmt.Errorf("bitfield too small: got %d; need %d", size, len(bits)*8)
 	}
 	copy(bf[start:], bits)
-	return bf
+	return bf, nil
 }
 
 func (bf Bitfield) offset(i int) (uint, uint8) {
 	return uint(len(bf)) - (uint(i) / 8) - 1, uint8(i) % 8
 }
diff --git a/bitfield_test.go b/bitfield_test.go
index 5f51382..0ed7f91 100644
--- a/bitfield_test.go
+++ b/bitfield_test.go
@@ -7,11 +7,12 @@ import (
 	"math/bits"
 	"testing"
 )
 
 func TestExhaustive24(t *testing.T) {
-	bf := NewBitfield(24)
+	bf, err := NewBitfield(24)
+	assertNoError(t, err)
 	max := 1 << 24
 
 	bint := new(big.Int)
 
 	bts := make([]byte, 4)
@@ -56,11 +57,12 @@ func TestExhaustive24(t *testing.T) {
 		}
 	}
 }
 
 func TestBitfield(t *testing.T) {
-	bf := NewBitfield(128)
+	bf, err := NewBitfield(128)
+	assertNoError(t, err)
 	if bf.OnesBefore(20) != 0 {
 		t.Fatal("expected no bits set")
 	}
 	bf.SetBit(10)
 	if bf.OnesBefore(20) != 1 {
@@ -89,14 +91,24 @@ func TestBitfield(t *testing.T) {
 			t.Fatalf("expected bit %d to be %v", i, bf.Bit(i))
 		}
 	}
 }
 
+func TestBadSizeFails(t *testing.T) {
+	for _, size := range [...]int{-8, 2, 1337, -3} {
+		_, err := NewBitfield(size)
+		if err == nil {
+			t.Fatalf("missing error for %d sized bitfield", size)
+		}
+	}
+}
+
 var benchmarkSize = 512
 
 func BenchmarkBitfield(t *testing.B) {
-	bf := NewBitfield(benchmarkSize)
+	bf, err := NewBitfield(benchmarkSize)
+	assertNoError(t, err)
 	t.ResetTimer()
 	for i := 0; i < t.N; i++ {
 		if bf.Bit(i % benchmarkSize) {
 			t.Fatal("bad", i)
 		}
@@ -121,34 +133,37 @@ func BenchmarkBitfield(t *testing.B) {
 			t.Fatal("bad", i)
 		}
 	}
 }
 
-func BenchmarkOnes(t *testing.B) {
-	bf := NewBitfield(benchmarkSize)
-	t.ResetTimer()
-	for i := 0; i < t.N; i++ {
+func BenchmarkOnes(b *testing.B) {
+	bf, err := NewBitfield(benchmarkSize)
+	assertNoError(b, err)
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
 		for j := 0; j*4 < benchmarkSize; j++ {
 			if bf.Ones() != j {
-				t.Fatal("bad", i)
+				b.Fatal("bad", i)
 			}
 			bf.SetBit(j * 4)
 		}
 		for j := 0; j*4 < benchmarkSize; j++ {
 			bf.UnsetBit(j * 4)
 		}
 	}
 }
 
-func BenchmarkBytes(t *testing.B) {
-	bfa := NewBitfield(211)
-	bfb := NewBitfield(211)
-	for j := 0; j*4 < 211; j++ {
+func BenchmarkBytes(b *testing.B) {
+	bfa, err := NewBitfield(216)
+	assertNoError(b, err)
+	bfb, err := NewBitfield(216)
+	assertNoError(b, err)
+	for j := 0; j*4 < 216; j++ {
 		bfa.SetBit(j * 4)
 	}
-	t.ResetTimer()
-	for i := 0; i < t.N; i++ {
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
 		bfb.SetBytes(bfa.Bytes())
 	}
 }
 
 func BenchmarkBigInt(t *testing.B) {
@@ -178,5 +193,20 @@ func BenchmarkBigInt(t *testing.B) {
 		if bint.Bit(i%benchmarkSize) != 0 {
 			t.Fatal("bad")
 		}
 	}
 }
+
+func FuzzFromBytes(f *testing.F) {
+	f.Fuzz(func(_ *testing.T, size int, bytes []byte) {
+		if size > 1<<20 { // We relly on consumers for limit checks, hopefully they understand that a New... factory allocates memory.
+			return
+		}
+		FromBytes(size, bytes)
+	})
+}
+
+func assertNoError(t testing.TB, e error) {
+	if e != nil {
+		t.Fatal(e)
+	}
+}

Vulnerable code: //
// Panics if size is not a multiple of 8.
func NewBitfield(size int) Bitfield {
		panic("Bitfield size must be a multiple of 8")
	return make([]byte, size/8)
func FromBytes(size int, bits []byte) Bitfield {
	bf := NewBitfield(size)
		panic("bitfield too small")
	return bf
	bf := NewBitfield(24)
	bf := NewBitfield(128)
	bf := NewBitfield(benchmarkSize)
func BenchmarkOnes(t *testing.B) {
	bf := NewBitfield(benchmarkSize)
	t.ResetTimer()
	for i := 0; i < t.N; i++ {
				t.Fatal("bad", i)
func BenchmarkBytes(t *testing.B) {
	bfa := NewBitfield(211)
	bfb := NewBitfield(211)
	for j := 0; j*4 < 211; j++ {
	t.ResetTimer()
	for i := 0; i < t.N; i++ {
Fixed code: 	"fmt"
func NewBitfield(size int) (Bitfield, error) {
	if size < 0 {
		return nil, fmt.Errorf("bitfield size must be positive; got %d", size)
	}
		return nil, fmt.Errorf("bitfield size must be a multiple of 8; got %d", size)
	return make([]byte, size/8), nil
func FromBytes(size int, bits []byte) (Bitfield, error) {
	bf, err := NewBitfield(size)
	if err != nil {
		return nil, err
	}
		return nil, fmt.Errorf("bitfield too small: got %d; need %d", size, len(bits)*8)
	return bf, nil
	bf, err := NewBitfield(24)
	assertNoError(t, err)
	bf, err := NewBitfield(128)
	assertNoError(t, err)
func TestBadSizeFails(t *testing.T) {
	for _, size := range [...]int{-8, 2, 1337, -3} {
		_, err := NewBitfield(size)
		if err == nil {
			t.Fatalf("missing error for %d sized bitfield", size)
		}
	}
}

	bf, err := NewBitfield(benchmarkSize)
	assertNoError(t, err)
func BenchmarkOnes(b *testing.B) {
	bf, err := NewBitfield(benchmarkSize)
	assertNoError(b, err)
	b.ResetTimer()
	for i := 0; i < b.N; i++ {
				b.Fatal("bad", i)
func BenchmarkBytes(b *testing.B) {
	bfa, err := NewBitfield(216)
	assertNoError(b, err)
	bfb, err := NewBitfield(216)
	assertNoError(b, err)
	for j := 0; j*4 < 216; j++ {
	b.ResetTimer()
	for i := 0; i < b.N; i++ {

func FuzzFromBytes(f *testing.F) {
	f.Fuzz(func(_ *testing.T, size int, bytes []byte) {
		if size > 1<<20 { // We relly on consumers for limit checks, hopefully they understand that a New... factory allocates memory.
			return
		}
		FromBytes(size, bytes)
	})
}

func assertNoError(t testing.TB, e error) {
	if e != nil {
		t.Fatal(e)
	}
}