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. diff --git a/geom/twkb_parser.go b/geom/twkb_parser.go index cb94b2fd..34da1568 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 } } @@ -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() 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) + }) }