geom: validate TWKB ID list count before narrowing to int

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.

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
 		}
 	}
@@ -579,10 +579,10 @@ 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(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()

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)
+	})
 }