geom: fix panic in UnmarshalTWKB on huge element counts

[ChrisJr404]

Description

UnmarshalTWKB panics on certain malformed inputs. TWKB element counts (a
LineString/ring point count, or a MultiPoint/collection ID list length) are
read as untrusted uvarints and passed straight into make() as the slice
length. A crafted count of 2^64-1 casts to a negative int and panics with
runtime error: makeslice: len out of range; smaller-but-still-huge counts
instead trigger an oversized allocation. Both are reachable directly from the
documented UnmarshalTWKB entry point with a handful of bytes, so any caller
decoding untrusted TWKB can be crashed.

Minimal repro (before this change):

// LineString, precision 0, no flags, then a 2^64-1 point count.
in := []byte{0x02, 0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01}
geom.UnmarshalTWKB(in) // panic: makeslice: len out of range

The fix validates each count against the number of remaining bytes before
allocating, in parsePointArray and parseIDList. Every coordinate and every
ID is encoded as a varint of at least one byte, so a count larger than the
remaining buffer (or negative, from the overflow cast) cannot describe a valid
TWKB and is now rejected with an error. This mirrors the length checks the WKB
parser already performs. Valid inputs are unaffected — the full geom test
suite passes.

Check List

Have you:

• Added unit tests? Yes — TestUnmarshalTWKBHugeCount covers the LineString,
  Polygon-ring, and MultiPoint-ID-list paths. It panics before this change and
  passes after.

• Add cmprefimpl tests? Not appropriate (this is a malformed-input rejection,
  not a value-semantics change).

• Updated release notes? Yes — added a bullet to the Unreleased section of
  CHANGELOG.md.

• Updated the README.md? Not applicable (no new functionality).

Related Issue

• None; found while reading the TWKB parser.

Benchmark Results

• Not applicable; the change only adds a bounds check on the malformed-input
  path.

[peterstace]

Thanks for contributing @ChrisJr404 !

I've got a few small requests to clean some things up before merging.

[peterstace]

parsePointCountAndArray narrows the count before this check, passing int(numPoints), so 2^64-1 arrives here as -1. That's what the < 0 arm is for, and the error then reads number of points -1 exceeds remaining buffer size. wkb_parser.go:231 compares in uint64 before narrowing and so needs no negative case:

Suggested change

	func (p *twkbParser) parsePointArray(numPoints int) ([]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 numPoints points needs at least numPoints*dimensions
	// remaining bytes. Checking this before allocating avoids a make() panic
	// (or excessive memory allocation) driven by an untrusted count.
	if numPoints < 0 || numPoints > (len(p.twkb)-p.pos)/p.dimensions {
	return nil, fmt.Errorf("number of points %d exceeds remaining buffer size", numPoints)
	}
	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)

parsePointCountAndArray then drops its int(...) cast.

[peterstace]

The file already imports internal/test and uses it throughout, and test.Err does exactly this check.

Suggested change

	if _, err := geom.UnmarshalTWKB(tc.twkb); err == nil {
	t.Fatal("expected an error, but got nil")
	}
	_, err := geom.UnmarshalTWKB(tc.twkb)
	test.Err(t, err)

[ChrisJr404]

Thanks for the review. I made both cleanups: parsePointArray now takes the count as a uint64 and checks it before narrowing to int (so the negative arm is gone and the caller drops its cast), and the huge-count test uses test.Err now.

[peterstace]

LGTM, thanks for the contribution!