From 669ba2b3460b4691fe8df1cb5a73c9f02b9b3e22 Mon Sep 17 00:00:00 2001 From: Peter Stace Date: Thu, 20 Aug 2026 13:41:24 +1000 Subject: [PATCH 1/3] geom: validate TWKB ID list count before narrowing to int parseIDList took its element count as an int, so it needed a numIDs < 0 arm to catch counts that had already wrapped, and its error reported the wrapped negative value rather than the count given in the input. Take the count as a uint64 and bounds-check it before narrowing, matching parsePointArray. The multiplier is 1 rather than the dimension count, since each ID is a single signed varint of at least one byte. The error now reports the true count and the number of remaining bytes. Also cover UnmarshalTWKBIDList, the second exported entry point into this guard, in TestUnmarshalTWKBHugeCount. --- geom/twkb_parser.go | 25 +++++++++++++------------ geom/twkb_test.go | 14 ++++++++++---- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/geom/twkb_parser.go b/geom/twkb_parser.go index cb94b2fd..66a404a6 100644 --- a/geom/twkb_parser.go +++ b/geom/twkb_parser.go @@ -48,7 +48,7 @@ func UnmarshalTWKBIDList(twkb []byte) ([]int64, bool, error) { return nil, false, p.annotateError(fmt.Errorf("ID list size uvarint malformed: %w", err)) } - if err := p.parseIDList(int(numItems)); err != nil { + if err := p.parseIDList(numItems); err != nil { return nil, false, p.annotateError(err) } return p.idList, true, nil @@ -498,7 +498,7 @@ func (p *twkbParser) nextMultiPoint() (MultiPoint, error) { return MultiPoint{}, fmt.Errorf("num points varint malformed: %w", err) } if p.hasIDs { - if err := p.parseIDList(int(numPoints)); err != nil { + if err := p.parseIDList(numPoints); err != nil { return MultiPoint{}, err } } @@ -526,7 +526,7 @@ func (p *twkbParser) nextMultiLineString() (MultiLineString, error) { return MultiLineString{}, fmt.Errorf("num linestrings varint malformed: %w", err) } if p.hasIDs { - if err := p.parseIDList(int(numLineStrings)); err != nil { + if err := p.parseIDList(numLineStrings); err != nil { return MultiLineString{}, err } } @@ -554,7 +554,7 @@ func (p *twkbParser) nextMultiPolygon() (MultiPolygon, error) { return MultiPolygon{}, fmt.Errorf("num polygons varint malformed: %w", err) } if p.hasIDs { - if err := p.parseIDList(int(numPolygons)); err != nil { + if err := p.parseIDList(numPolygons); err != nil { return MultiPolygon{}, err } } @@ -582,7 +582,7 @@ func (p *twkbParser) nextGeometryCollection() (GeometryCollection, error) { return GeometryCollection{}, fmt.Errorf("num polygons varint malformed: %w", err) } if p.hasIDs { - if err := p.parseIDList(int(numGeoms)); err != nil { + if err := p.parseIDList(numGeoms); err != nil { return GeometryCollection{}, err } } @@ -644,15 +644,16 @@ func (p *twkbParser) parsePointArray(count uint64) ([]float64, error) { return coords, nil } -func (p *twkbParser) parseIDList(numIDs int) error { +func (p *twkbParser) parseIDList(count uint64) error { // Guard against corrupt or malicious inputs that specify a huge ID count. - // Each ID is encoded as a varint of at least one byte, so a valid ID list - // needs at least numIDs remaining bytes. Checking this before allocating - // avoids a make() panic (or excessive memory allocation) driven by an - // untrusted count. - if numIDs < 0 || numIDs > len(p.twkb)-p.pos { - return fmt.Errorf("number of IDs %d exceeds remaining buffer size", numIDs) + // Each ID is encoded as a varint of at least one byte, so a valid encoding + // of count IDs needs at least count remaining bytes. Checking the count + // before narrowing it to an int keeps an untrusted value out of make(). + remaining := len(p.twkb) - p.pos + if count > uint64(remaining) { + return fmt.Errorf("number of IDs %d exceeds remaining buffer size of %d bytes", count, remaining) } + numIDs := int(count) p.idList = make([]int64, numIDs) for i := 0; i < numIDs; i++ { id, err := p.parseSignedVarint() diff --git a/geom/twkb_test.go b/geom/twkb_test.go index 3d357692..8ecc3cf7 100644 --- a/geom/twkb_test.go +++ b/geom/twkb_test.go @@ -544,10 +544,9 @@ func minMax(a, b float64) (float64, float64) { // TestUnmarshalTWKBHugeCount checks that TWKBs specifying an element count that // is wildly larger than the remaining buffer are rejected with an error rather -// than causing a panic (or an attempt at an enormous allocation). The counts -// are attacker-controlled varints, so without a bound check a value such as -// 2^64-1 casts to a negative int and panics make() with "makeslice: len out of -// range". +// than causing a panic (or an attempt at an enormous allocation). The counts are +// attacker-controlled varints used to size allocations, so a value such as +// 2^64-1 must be rejected before it reaches make(). func TestUnmarshalTWKBHugeCount(t *testing.T) { // A uvarint encoding of 2^64-1 (ten bytes). maxUvarint := []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01} @@ -577,4 +576,11 @@ func TestUnmarshalTWKBHugeCount(t *testing.T) { test.Err(t, err) }) } + + t.Run("id list entry point", func(t *testing.T) { + // MultiPoint, precision 0, ID list flag set, huge ID/point count. + twkb := append([]byte{0x04, 0x04}, maxUvarint...) + _, _, err := geom.UnmarshalTWKBIDList(twkb) + test.Err(t, err) + }) } From 9968ecf0617ed1b59053c18b4679eca2898b8183 Mon Sep 17 00:00:00 2001 From: Peter Stace Date: Fri, 21 Aug 2026 09:30:24 +1000 Subject: [PATCH 2/3] geom: report geometry count in GeometryCollection varint error The malformed-varint error in nextGeometryCollection said "num polygons" while reading the collection's geometry count. Every other collection parser names its own element type. --- geom/twkb_parser.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/geom/twkb_parser.go b/geom/twkb_parser.go index 66a404a6..34da1568 100644 --- a/geom/twkb_parser.go +++ b/geom/twkb_parser.go @@ -579,7 +579,7 @@ func (p *twkbParser) parseGeometryCollection() (GeometryCollection, error) { func (p *twkbParser) nextGeometryCollection() (GeometryCollection, error) { numGeoms, err := p.parseUnsignedVarint() if err != nil { - return GeometryCollection{}, fmt.Errorf("num polygons varint malformed: %w", err) + return GeometryCollection{}, fmt.Errorf("num geometries varint malformed: %w", err) } if p.hasIDs { if err := p.parseIDList(numGeoms); err != nil { From 1425df85519ab7817a7c95906af0c246fd25c9d5 Mon Sep 17 00:00:00 2001 From: Peter Stace Date: Fri, 21 Aug 2026 10:11:48 +1000 Subject: [PATCH 3/3] Loosen the CHANGELOG guideline in CLAUDE.md The guideline required an entry whenever a change was visible to users of the module, which is stricter than the CHANGELOG's own history: it records internal work that is not externally detectable, and it records nothing about the wording of error messages. State what warrants an entry, note that internal work can be logged, and name the cases too small to bother with. --- CLAUDE.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index f4d4d17d..d8a5570c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,3 +1,11 @@ # CLAUDE.md -Update CHANGELOG.md whenever making a change visible to users of this module. +Update CHANGELOG.md when a change alters what callers of this module can +observe. That covers API that is added, removed, or renamed, changed +behaviour or results, bug fixes, and performance work. Notable internal work +can also be logged, saying that it is not externally detectable. + +Skip the entry when nothing a caller could depend on has changed, such as the +wording of an error message, comments, or test-only changes. Where a change +refines something already listed under Unreleased, extend that entry rather +than adding a second entry.