Skip to content

Commit cc2ebd9

Browse files
jackcclaude
andcommitted
Return errors instead of panicking on malformed binary records
The binary record header carries a signed int32 field count read straight off the wire, and RecordCodec.DecodeValue used it to size the value slice. Three index panics followed from trusting it: - A negative count reached make([]any, count) and panicked with "makeslice: len out of range". An oversized count requested an allocation unrelated to the data actually present. - A count that understated the number of fields on the wire ran values[i] past the end of the presized slice. - A source with more fields than the destination CompositeFields has slots indexed past the end of that slice. This one needs no malformed input at all: scanning "select row('foo'::text, 42::int4)" into a one-element CompositeFields panicked against a stock server. Validate the count in NewCompositeBinaryScanner the way MultirangeCodec.decodeBinary validates its element count, bounding it against the bytes available (each field needs at least 8 for its OID and length prefix). Append the values actually scanned rather than indexing into a presized slice, and report a field count mismatch against CompositeFields as an error from both the record and composite scan plans. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 9656329 commit cc2ebd9

3 files changed

Lines changed: 208 additions & 7 deletions

File tree

pgtype/composite.go

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,17 @@ func (plan *scanPlanBinaryCompositeToCompositeIndexScanner) Scan(src []byte, tar
139139
scanner := NewCompositeBinaryScanner(plan.m, src)
140140
for i, field := range plan.cc.Fields {
141141
if scanner.Next() {
142-
fieldTarget := targetScanner.ScanIndex(i)
142+
fieldTarget, err := compositeFieldTarget(targetScanner, i)
143+
if err != nil {
144+
return err
145+
}
143146
if fieldTarget != nil {
144147
fieldPlan := plan.m.PlanScan(field.Type.OID, BinaryFormatCode, fieldTarget)
145148
if fieldPlan == nil {
146149
return fmt.Errorf("unable to encode %v into OID %d in binary format", field, field.Type.OID)
147150
}
148151

149-
err := fieldPlan.Scan(scanner.Bytes(), fieldTarget)
152+
err = fieldPlan.Scan(scanner.Bytes(), fieldTarget)
150153
if err != nil {
151154
return err
152155
}
@@ -297,6 +300,15 @@ func NewCompositeBinaryScanner(m *Map, src []byte) *CompositeBinaryScanner {
297300
fieldCount := int32(binary.BigEndian.Uint32(src[rp:]))
298301
rp += 4
299302

303+
if fieldCount < 0 {
304+
return &CompositeBinaryScanner{err: fmt.Errorf("Record field count %d is negative", fieldCount)}
305+
}
306+
307+
// Each field requires at least 8 bytes: 4 for the OID and 4 for the length prefix.
308+
if int(fieldCount) > len(src[rp:])/8 {
309+
return &CompositeBinaryScanner{err: fmt.Errorf("Record field count %d exceeds available data", fieldCount)}
310+
}
311+
300312
return &CompositeBinaryScanner{
301313
m: m,
302314
rp: rp,
@@ -344,6 +356,17 @@ func (cfs *CompositeBinaryScanner) FieldCount() int {
344356
return int(cfs.fieldCount)
345357
}
346358

359+
// compositeFieldTarget returns the scan target for field i. CompositeFields is
360+
// a slice, so indexing past its end would panic. A source with more fields than
361+
// the destination is a mismatch that must be reported as an error.
362+
func compositeFieldTarget(targetScanner CompositeIndexScanner, i int) (any, error) {
363+
if cf, ok := targetScanner.(CompositeFields); ok && i >= len(cf) {
364+
return nil, fmt.Errorf("cannot scan composite field %d into CompositeFields of length %d", i, len(cf))
365+
}
366+
367+
return targetScanner.ScanIndex(i), nil
368+
}
369+
347370
// Bytes returns the bytes of the field most recently read by Scan().
348371
func (cfs *CompositeBinaryScanner) Bytes() []byte {
349372
return cfs.fieldBytes

pgtype/record_codec.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,14 +48,17 @@ func (plan *scanPlanBinaryRecordToCompositeIndexScanner) Scan(src []byte, target
4848

4949
scanner := NewCompositeBinaryScanner(plan.m, src)
5050
for i := 0; scanner.Next(); i++ {
51-
fieldTarget := targetScanner.ScanIndex(i)
51+
fieldTarget, err := compositeFieldTarget(targetScanner, i)
52+
if err != nil {
53+
return err
54+
}
5255
if fieldTarget != nil {
5356
fieldPlan := plan.m.PlanScan(scanner.OID(), BinaryFormatCode, fieldTarget)
5457
if fieldPlan == nil {
5558
return fmt.Errorf("unable to scan OID %d in binary format into %v", scanner.OID(), fieldTarget)
5659
}
5760

58-
err := fieldPlan.Scan(scanner.Bytes(), fieldTarget)
61+
err = fieldPlan.Scan(scanner.Bytes(), fieldTarget)
5962
if err != nil {
6063
return err
6164
}
@@ -96,8 +99,11 @@ func (RecordCodec) DecodeValue(m *Map, oid uint32, format int16, src []byte) (an
9699
return string(src), nil
97100
case BinaryFormatCode:
98101
scanner := NewCompositeBinaryScanner(m, src)
99-
values := make([]any, scanner.FieldCount())
100-
for i := 0; scanner.Next(); i++ {
102+
// The field count is a hint for the initial allocation only. Append the
103+
// values actually present rather than indexing into a presized slice, as
104+
// the source may carry more or fewer fields than the header claims.
105+
values := make([]any, 0, scanner.FieldCount())
106+
for scanner.Next() {
101107
var v any
102108
fieldPlan := m.PlanScan(scanner.OID(), BinaryFormatCode, &v)
103109
if fieldPlan == nil {
@@ -109,7 +115,7 @@ func (RecordCodec) DecodeValue(m *Map, oid uint32, format int16, src []byte) (an
109115
return nil, err
110116
}
111117

112-
values[i] = v
118+
values = append(values, v)
113119
}
114120

115121
if err := scanner.Err(); err != nil {

pgtype/record_malformed_test.go

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
package pgtype_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/jackc/pgx/v5/pgtype"
7+
)
8+
9+
// int4Field builds the binary wire encoding of one non-NULL int4 record field.
10+
func int4Field(v byte) []byte {
11+
return []byte{0x00, 0x00, 0x00, 0x17, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, v}
12+
}
13+
14+
// A binary record header carries a signed int32 field count taken straight from
15+
// the wire. A negative count reached make([]any, count) and panicked with
16+
// "makeslice: len out of range", and an oversized count requested an allocation
17+
// unrelated to the amount of data actually present.
18+
func TestRecordBinaryDecodeMalformedFieldCountReturnsError(t *testing.T) {
19+
m := pgtype.NewMap()
20+
dt, ok := m.TypeForOID(pgtype.RecordOID)
21+
if !ok {
22+
t.Fatal("no type registered for record OID")
23+
}
24+
25+
for _, tt := range []struct {
26+
name string
27+
src []byte
28+
}{
29+
{"negative", []byte{0xff, 0xff, 0xff, 0xff}},
30+
{"negative min", []byte{0x80, 0x00, 0x00, 0x00}},
31+
{"oversized", []byte{0x7f, 0xff, 0xff, 0xff}},
32+
{"count exceeds data", []byte{0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x17, 0xff, 0xff, 0xff, 0xff}},
33+
} {
34+
t.Run(tt.name, func(t *testing.T) {
35+
// A panic here (pre-fix) fails the test.
36+
if _, err := dt.Codec.DecodeValue(m, pgtype.RecordOID, pgtype.BinaryFormatCode, tt.src); err == nil {
37+
t.Errorf("DecodeValue(% x) = nil error, want an error", tt.src)
38+
}
39+
})
40+
}
41+
}
42+
43+
// A field count that understates how many fields follow must not run past the
44+
// end of the presized value slice.
45+
func TestRecordBinaryDecodeFieldCountUnderstated(t *testing.T) {
46+
m := pgtype.NewMap()
47+
dt, ok := m.TypeForOID(pgtype.RecordOID)
48+
if !ok {
49+
t.Fatal("no type registered for record OID")
50+
}
51+
52+
for _, tt := range []struct {
53+
name string
54+
src []byte
55+
want int
56+
}{
57+
{"count 0, one field", append([]byte{0x00, 0x00, 0x00, 0x00}, int4Field(1)...), 1},
58+
{"count 1, two fields", append(append([]byte{0x00, 0x00, 0x00, 0x01}, int4Field(1)...), int4Field(2)...), 2},
59+
} {
60+
t.Run(tt.name, func(t *testing.T) {
61+
// A panic here (pre-fix) fails the test.
62+
v, err := dt.Codec.DecodeValue(m, pgtype.RecordOID, pgtype.BinaryFormatCode, tt.src)
63+
if err != nil {
64+
t.Fatalf("unexpected error: %v", err)
65+
}
66+
values, ok := v.([]any)
67+
if !ok {
68+
t.Fatalf("got %T, want []any", v)
69+
}
70+
if len(values) != tt.want {
71+
t.Errorf("got %d values (%v), want %d", len(values), values, tt.want)
72+
}
73+
})
74+
}
75+
}
76+
77+
// Scanning a row with more fields than the destination CompositeFields has
78+
// slots indexed past the end of the slice and panicked. This needs no malformed
79+
// input — "select row('foo'::text, 42::int4)" into a one-element
80+
// CompositeFields is enough.
81+
func TestRecordScanUndersizedCompositeFieldsReturnsError(t *testing.T) {
82+
m := pgtype.NewMap()
83+
84+
src := append(append([]byte{0x00, 0x00, 0x00, 0x02}, int4Field(1)...), int4Field(2)...)
85+
86+
var a int32
87+
dst := pgtype.CompositeFields{&a}
88+
89+
plan := m.PlanScan(pgtype.RecordOID, pgtype.BinaryFormatCode, dst)
90+
// A panic here (pre-fix) fails the test.
91+
if err := plan.Scan(src, dst); err == nil {
92+
t.Error("Scan into undersized CompositeFields = nil error, want an error")
93+
}
94+
}
95+
96+
// The composite codec reaches the same destination indexing through its own
97+
// scan plan.
98+
func TestCompositeScanUndersizedCompositeFieldsReturnsError(t *testing.T) {
99+
m := pgtype.NewMap()
100+
int4Type, ok := m.TypeForOID(pgtype.Int4OID)
101+
if !ok {
102+
t.Fatal("no type registered for int4 OID")
103+
}
104+
105+
const compositeOID = 100000
106+
m.RegisterType(&pgtype.Type{
107+
Name: "test_composite",
108+
OID: compositeOID,
109+
Codec: &pgtype.CompositeCodec{Fields: []pgtype.CompositeCodecField{
110+
{Name: "a", Type: int4Type},
111+
{Name: "b", Type: int4Type},
112+
}},
113+
})
114+
115+
src := append(append([]byte{0x00, 0x00, 0x00, 0x02}, int4Field(1)...), int4Field(2)...)
116+
117+
var a int32
118+
dst := pgtype.CompositeFields{&a}
119+
120+
plan := m.PlanScan(compositeOID, pgtype.BinaryFormatCode, dst)
121+
// A panic here (pre-fix) fails the test.
122+
if err := plan.Scan(src, dst); err == nil {
123+
t.Error("Scan into undersized CompositeFields = nil error, want an error")
124+
}
125+
}
126+
127+
// Well-formed records still decode after the guards were added.
128+
func TestRecordBinaryDecodeValid(t *testing.T) {
129+
m := pgtype.NewMap()
130+
dt, ok := m.TypeForOID(pgtype.RecordOID)
131+
if !ok {
132+
t.Fatal("no type registered for record OID")
133+
}
134+
135+
for _, tt := range []struct {
136+
name string
137+
src []byte
138+
want []any
139+
}{
140+
{"empty", []byte{0x00, 0x00, 0x00, 0x00}, []any{}},
141+
{"one field", append([]byte{0x00, 0x00, 0x00, 0x01}, int4Field(1)...), []any{int32(1)}},
142+
{
143+
"two fields",
144+
append(append([]byte{0x00, 0x00, 0x00, 0x02}, int4Field(1)...), int4Field(2)...),
145+
[]any{int32(1), int32(2)},
146+
},
147+
{
148+
"null field",
149+
[]byte{0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x17, 0xff, 0xff, 0xff, 0xff},
150+
[]any{nil},
151+
},
152+
} {
153+
t.Run(tt.name, func(t *testing.T) {
154+
v, err := dt.Codec.DecodeValue(m, pgtype.RecordOID, pgtype.BinaryFormatCode, tt.src)
155+
if err != nil {
156+
t.Fatalf("unexpected error: %v", err)
157+
}
158+
values, ok := v.([]any)
159+
if !ok {
160+
t.Fatalf("got %T, want []any", v)
161+
}
162+
if len(values) != len(tt.want) {
163+
t.Fatalf("got %v, want %v", values, tt.want)
164+
}
165+
for i := range tt.want {
166+
if values[i] != tt.want[i] {
167+
t.Errorf("field %d: got %v, want %v", i, values[i], tt.want[i])
168+
}
169+
}
170+
})
171+
}
172+
}

0 commit comments

Comments
 (0)