ayush00git opened a new issue, #3612: URL: https://github.com/apache/fory/issues/3612
### Search before asking - [x] I had searched in the [issues](https://github.com/apache/fory/issues) and found no similar issues. ### 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: ```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? - [x] I'm willing to submit a PR! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
