diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cf33085..5bcfe79d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +- Fix a panic in `UnmarshalTWKB` when parsing a TWKB whose element count (point + count, ring point count, or ID list length) is far larger than the remaining + input. These counts are untrusted varints, so a crafted value would cause an + out-of-range `make` and panic (or trigger an excessive allocation). The + counts are now validated against the number of remaining bytes before + allocating. + - Add `NewEnvelopeXY` constructor, which builds an `Envelope` from variadic x and y coordinates (x1, y1, x2, y2, ..., xn, yn), where `NewEnvelope` takes `XY` values. It follows the same convention as the other `XY` constructors, diff --git a/geom/twkb_parser.go b/geom/twkb_parser.go index 125d8087..cb94b2fd 100644 --- a/geom/twkb_parser.go +++ b/geom/twkb_parser.go @@ -609,14 +609,24 @@ func (p *twkbParser) parsePointCountAndArray() ([]float64, int, error) { return nil, 0, fmt.Errorf("num points varint malformed: %w", err) } - coords, err := p.parsePointArray(int(numPoints)) + coords, err := p.parsePointArray(numPoints) return coords, int(numPoints), err } // Convert a given number of points from integer to floating point coordinates. // Utilise and update the running memory of the previous reference point. // The returned array will contain numPoints * the number of dimensions values. -func (p *twkbParser) parsePointArray(numPoints int) ([]float64, error) { +func (p *twkbParser) parsePointArray(count uint64) ([]float64, error) { + // Guard against corrupt or malicious inputs that specify a huge point + // count. Each coordinate is encoded as a varint of at least one byte, so a + // valid encoding of count points needs at least count*dimensions 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/p.dimensions) { + return nil, fmt.Errorf("number of points %d exceeds remaining buffer size of %d bytes", count, remaining) + } + numPoints := int(count) coords := make([]float64, numPoints*p.dimensions) c := 0 for i := 0; i < numPoints; i++ { @@ -635,6 +645,14 @@ func (p *twkbParser) parsePointArray(numPoints int) ([]float64, error) { } func (p *twkbParser) parseIDList(numIDs int) 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) + } 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 230651ec..3d357692 100644 --- a/geom/twkb_test.go +++ b/geom/twkb_test.go @@ -541,3 +541,40 @@ func TestZigZagInt(t *testing.T) { func minMax(a, b float64) (float64, float64) { return math.Min(a, b), math.Max(a, b) } + +// 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". +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} + for _, tc := range []struct { + description string + twkb []byte + }{ + { + // LineString, precision 0, no metadata flags, huge point count. + description: "linestring point count", + twkb: append([]byte{0x02, 0x00}, maxUvarint...), + }, + { + // Polygon, precision 0, no metadata flags, one ring with a huge + // point count. + description: "polygon ring point count", + twkb: append([]byte{0x03, 0x00, 0x01}, maxUvarint...), + }, + { + // MultiPoint, precision 0, ID list flag set, huge ID/point count. + description: "multipoint id list count", + twkb: append([]byte{0x04, 0x04}, maxUvarint...), + }, + } { + t.Run(tc.description, func(t *testing.T) { + _, err := geom.UnmarshalTWKB(tc.twkb) + test.Err(t, err) + }) + } +}