Skip to content

Commit bb2723d

Browse files
authored
Merge commit from fork
`int(r)` would wrap negative for r > MaxInt64, slipping past the bounds check and causing an OOB read in encode. Fix & add regression test.
1 parent 539243b commit bb2723d

2 files changed

Lines changed: 35 additions & 2 deletions

File tree

s2/dict.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,12 @@ func NewDict(dict []byte) *Dict {
5656
if len(dict) < MinDictSize || len(dict) > MaxDictSize {
5757
return nil
5858
}
59-
d.repeat = int(r)
60-
if d.repeat > len(dict) {
59+
// Compare as uint64: int(r) would wrap negative for r > MaxInt64,
60+
// slipping past the bounds check and causing an OOB read in encode.
61+
if r > uint64(len(dict)) {
6162
return nil
6263
}
64+
d.repeat = int(r)
6365
return &d
6466
}
6567

s2/dict_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,10 @@ import (
99
"bytes"
1010
"compress/gzip"
1111
"encoding/base64"
12+
"encoding/binary"
1213
"fmt"
1314
"io"
15+
"math"
1416
"math/rand"
1517
"os"
1618
"strings"
@@ -102,6 +104,35 @@ func TestDict(t *testing.T) {
102104
}
103105
}
104106

107+
func TestNewDictRepeatOverflow(t *testing.T) {
108+
// A repeat value that doesn't fit in the dict must be rejected before the
109+
// int(r) cast: the cast wraps such values negative (CWE-190) — on 32-bit
110+
// for r >= 1<<31, on 64-bit for r > MaxInt64 — and a negative repeat slips
111+
// past the bounds check, later causing an OOB read in encodeBlockDictGo.
112+
for _, r := range []uint64{
113+
1 << 31, // wraps to a negative int32 on 32-bit platforms
114+
math.MaxUint32, // wraps to int32(-1) on 32-bit platforms
115+
math.MaxInt64 + 1, // wraps to a negative int64 on 64-bit platforms
116+
math.MaxUint64,
117+
12687765596934209639, // from the original report
118+
} {
119+
var buf [binary.MaxVarintLen64]byte
120+
n := binary.PutUvarint(buf[:], r)
121+
dictData := append(append([]byte{}, buf[:n]...), make([]byte, MinDictSize)...)
122+
if d := NewDict(dictData); d != nil {
123+
t.Errorf("repeat %d: expected nil dict, got repeat=%d", r, d.repeat)
124+
}
125+
}
126+
127+
// Boundary repeat == len(dict) must still be accepted (unchanged behavior).
128+
dict := make([]byte, MinDictSize)
129+
var buf [binary.MaxVarintLen64]byte
130+
n := binary.PutUvarint(buf[:], uint64(len(dict)))
131+
if NewDict(append(append([]byte{}, buf[:n]...), dict...)) == nil {
132+
t.Error("repeat == len(dict) should be accepted")
133+
}
134+
}
135+
105136
func TestDictBetter(t *testing.T) {
106137
rng := rand.New(rand.NewSource(1))
107138
data := make([]byte, 128<<10)

0 commit comments

Comments
 (0)