Skip to content

encoding/wkb: prevent panic on overflowing point count - #179

Open
ChrisJr404 wants to merge 1 commit into
paulmach:masterfrom
ChrisJr404:fix-wkb-point-count-overflow
Open

encoding/wkb: prevent panic on overflowing point count#179
ChrisJr404 wants to merge 1 commit into
paulmach:masterfrom
ChrisJr404:fix-wkb-point-count-overflow

Conversation

@ChrisJr404

Copy link
Copy Markdown

Summary

Unmarshal panics with an out-of-range slice index on some malformed WKB input. The header carries a 32-bit point count that is read straight from the bytes, and the length check that is supposed to reject truncated buffers overflows, so an undersized buffer slips through and the read loop indexes past the end of the slice.

In wkbcommon.unmarshalPoints:

num := unmarshalUint32(order, data)
data = data[4:]

if len(data) < int(num*16) {   // num*16 is computed in uint32 and can wrap
    return nil, ErrNotWKB
}

num is a uint32, so num * 16 is evaluated in uint32 and wraps modulo 2^32. A header claiming 0x10000001 points makes the product wrap to 16, so a 16-byte buffer passes the guard; the loop then runs int(num) (~268M) iterations and reads data[16*i:], panicking as soon as i walks off the end.

Reproducer (a 25-byte input):

data := []byte{
    0x01,                   // little endian
    0x02, 0x00, 0x00, 0x00, // type: linestring
    0x01, 0x00, 0x00, 0x10, // point count 0x10000001
}
data = append(data, make([]byte, 16)...) // one point's worth of data
_, _ = wkb.Unmarshal(data)               // panic: index out of range
panic: runtime error: index out of range [7] with length 0
encoding/binary.littleEndian.Uint64(...)
github.com/paulmach/orb/encoding/internal/wkbcommon.unmarshalPoints(...)

This is reachable from the documented wkb.Unmarshal / ewkb.Unmarshal entry points (linestrings and polygon rings both go through unmarshalPoints), so parsing untrusted WKB can crash the caller.

Fix

Do the multiplication in 64-bit space so the bounds check is accurate:

if uint64(len(data)) < uint64(num)*16 {
    return nil, ErrNotWKB
}

Valid input is unaffected — for any count that legitimately fits the buffer the comparison is identical. Truncated input now returns ErrNotWKB instead of panicking. Added a regression test, and go test ./... passes.

unmarshalPoints validated the buffer size with len(data) < int(num*16),
but num is a uint32 taken straight from the input so num*16 is computed in
uint32 and can wrap to a small value. A linestring (or polygon ring, or
ewkb geometry) header claiming e.g. 0x10000001 points made the product wrap
to 16, so an undersized buffer passed the guard and the read loop then
indexed past the end of the slice, panicking.

Do the multiplication in 64-bit space so the bounds check is accurate, and
add a regression test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant