Search before asking
Version
0.17.0
Component(s)
Go
Minimal reproduce step
The following self-contained Go test demonstrates the write-path issue using a sentinel-value check. It can be added to go/fory/buffer_test.go:
func TestUnsafePutVarUint32OOBWrite(t *testing.T) {
// Construct a ByteBuffer backed by a slice where len == writerIndex + 5.
// This replicates the condition where Reserve(5) is satisfied without expanding
// the buffer, but UnsafePutVarUint32 then writes 8 physical bytes.
const sentinelByte = byte(0xAB)
const totalCap = 16 // backing large enough to observe the overwrite safely
backing := make([]byte, totalCap, totalCap)
// Fill bytes [5, totalCap) with sentinel values.
for i := 5; i < totalCap; i++ {
backing[i] = sentinelByte
}
// Simulate struct.go's fast-path: Reserve(MaxVarintSize=5) on a tight buffer.
// Expose only 5 bytes of len so Reserve(5) returns immediately.
buf := NewByteBuffer(backing[:5]) // len=5, cap=16
// Replicate exactly what struct.go:342 does for a uint32 varint field:
buf.Reserve(5) // needed(5) <= len(5) → returns immediately, NO growth
// A uint32 value >= 2^28 encodes to 5 varint bytes → triggers 8-byte bulk write.
written := buf.UnsafePutVarUint32(0, 1<<28)
if written != 5 {
t.Fatalf("expected 5 bytes written, got %d", written)
}
// Bytes at indices 5, 6, 7 are past Reserve(5)'s guaranteed region.
// If they differ from the sentinel, the 8-byte write overflowed the reserved region.
overwritten := 0
for i := 5; i < 8; i++ {
if backing[i] != sentinelByte {
t.Logf("backing[%d] = 0x%02X (overwritten, was 0x%02X)", i, backing[i], sentinelByte)
overwritten++
}
}
if overwritten > 0 {
t.Errorf("UnsafePutVarUint32 wrote %d byte(s) past Reserve(5)'s guaranteed region "+
"(contract requires Reserve(8) per buffer.go:661)", overwritten)
}
}What did you expect to see?
- Expected: test passes — no bytes written past index 4.
What did you see instead?
- Actual: backing[5], backing[6], backing[7] are overwritten — 3 bytes past the reserved region.
Note on the test design: cap=16 is used intentionally so the overflow lands within the backing array and the test runs safely without corrupting other heap objects. In production, when a ByteBuffer is created from an inbound network slice via NewByteBuffer(inboundSlice) (where len == cap), those 3 bytes escape the Go allocation and corrupt adjacent heap memory.
Anything Else?
The Go runtime's struct serialization fast-path in struct.go violates the documented contract of UnsafePutVarUint32 and UnsafeReadVarUint32 in buffer.go. Specifically:
UnsafePutVarUint32 documents that the caller must call Reserve(8) (because it performs an 8-byte bulk write for 5-byte varints), but struct.go only calls Reserve(MaxVarintSize), which is 5 for uint32/int32 varint fields.UnsafeReadVarUint32 physically reads 8 bytes, but the fast-path guard in struct.go only checks remaining() >= MaxVarintSize (which can be 5).
This is a correctness bug / contract violation — the mismatch can cause writes past the end of a buffer's allocated memory in any situation where the buffer's capacity is not generously over-sized (e.g. when a ByteBuffer is constructed from a pre-existing []byte via NewByteBuffer).
Are you willing to submit a PR?
Search before asking
Version
0.17.0
Component(s)
Go
Minimal reproduce step
The following self-contained Go test demonstrates the write-path issue using a sentinel-value check. It can be added to go/fory/buffer_test.go:
What did you expect to see?
What did you see instead?
Note on the test design:
cap=16is used intentionally so the overflow lands within the backing array and the test runs safely without corrupting other heap objects. In production, when aByteBufferis created from an inbound network slice viaNewByteBuffer(inboundSlice)(wherelen == cap), those 3 bytes escape the Go allocation and corrupt adjacent heap memory.Anything Else?
The Go runtime's struct serialization fast-path in
struct.goviolates the documented contract ofUnsafePutVarUint32andUnsafeReadVarUint32inbuffer.go. Specifically:UnsafePutVarUint32documents that the caller must callReserve(8)(because it performs an 8-byte bulk write for 5-byte varints), butstruct.goonly callsReserve(MaxVarintSize), which is5foruint32/int32varint fields.UnsafeReadVarUint32physically reads 8 bytes, but the fast-path guard instruct.goonly checksremaining() >= MaxVarintSize(which can be 5).This is a correctness bug / contract violation — the mismatch can cause writes past the end of a buffer's allocated memory in any situation where the buffer's capacity is not generously over-sized (e.g. when a
ByteBufferis constructed from a pre-existing[]byteviaNewByteBuffer).Are you willing to submit a PR?