Add fuzz tests for the FFI buffer boundary

cobhan.go

@@ -103,12 +103,41 @@ func bufferPtrToDataPtr(bufferPtr unsafe.Pointer) unsafe.Pointer {
 }
 
 func bufferPtrToString(bufferPtr unsafe.Pointer, length C.int) string {
+	if length == 0 {
+		// bufferPtrToDataPtr(bufferPtr) would point exactly one byte past
+		// the end of a header-only allocation; forming that pointer via
+		// raw uintptr arithmetic (rather than Go's slice-indexing, which
+		// is allowed to compute one-past-the-end addresses) is invalid
+		// under checkptr, even though it's never dereferenced.
+		return ""
+	}
 	dataPtr := bufferPtrToDataPtr(bufferPtr)
 	//Allocation
 	return C.GoStringN((*C.char)(dataPtr), length)
 }
 
+// bufferPtrToBytes trusts length completely -- it comes from the buffer's
+// own header, which Cobhan's own write path (BytesToBuffer) always sets to
+// exactly the number of bytes actually written. There is no independent
+// capacity available here to validate that against: this function only
+// ever sees a raw pointer, and callers may be foreign (non-Go-allocated)
+// memory that Go's runtime has no visibility into at all. A header that
+// overstates the real buffer size (from a corrupted buffer, or a bug in
+// another language's Cobhan binding) will read past the end of the real
+// allocation with no error and no crash in a normal build -- this is an
+// inherent limitation of a length-prefixed wire format that trusts its own
+// length field, not something checkable from a bare pointer + length pair.
+// A fix would require changing this function's signature (and therefore
+// the C ABI every Cobhan binding depends on) to also carry a capacity.
 func bufferPtrToBytes(bufferPtr unsafe.Pointer, length C.int) ([]byte, int32) {
+	if length == 0 {
+		// See bufferPtrToString.
+		if copyBuffers {
+			return []byte{}, ERR_NONE
+		}
+		return nil, ERR_NONE
+	}
+
 	src := unsafe.Slice((*byte)(bufferPtrToDataPtr(bufferPtr)), length)
 
 	if copyBuffers {
@@ -130,6 +159,11 @@ func tempToBytes(ptr unsafe.Pointer, length C.int) ([]byte, int32) {
 	}
 
 	length = 0 - length
+	if length < 0 {
+		// Only math.MinInt32 negates to itself (two's complement overflow).
+		// There's no valid filename-length encoding for it.
+		return nil, ERR_READ_TEMP_FILE_FAILED
+	}
 
 	if bufferMaximum < int(length) {
 		return nil, ERR_BUFFER_TOO_LARGE
@@ -323,7 +357,13 @@ func BufferToJson(srcPtr unsafe.Pointer) (map[string]interface{}, int32) {
 	if err != nil {
 		return nil, ERR_JSON_DECODE_FAILED
 	}
-	return loadedJson.(map[string]interface{}), ERR_NONE
+	jsonMap, ok := loadedJson.(map[string]interface{})
+	if !ok {
+		// Valid JSON, but the top-level value isn't an object (e.g. an
+		// array, string, number, bool, or null).
+		return nil, ERR_JSON_DECODE_FAILED
+	}
+	return jsonMap, ERR_NONE
 }
 
 func BufferToJsonStruct(srcPtr unsafe.Pointer, dst interface{}) int32 {
@@ -399,8 +439,16 @@ func BytesToBuffer(bytes []byte, dstPtr unsafe.Pointer) int32 {
 	dstCapInt := int(dstCap)
 	bytesLen := len(bytes)
 
-	// Construct a byte slice out of the unsafe pointers
-	var dst []byte = unsafe.Slice((*byte)(bufferPtrToDataPtr(dstPtr)), dstCapInt)
+	// Construct a byte slice out of the unsafe pointers. A zero-capacity
+	// destination is header-only; bufferPtrToDataPtr(dstPtr) would point
+	// exactly one byte past the end of that allocation, which checkptr
+	// flags as invalid even though it's never dereferenced -- avoid
+	// forming it at all and just use a nil slice (copy(nil, ...) is a
+	// valid no-op).
+	var dst []byte
+	if dstCapInt > 0 {
+		dst = unsafe.Slice((*byte)(bufferPtrToDataPtr(dstPtr)), dstCapInt)
+	}
 	var result int
 	if dstCapInt < bytesLen {
 		// Output will not fit in supplied buffer

cobhan_test.go

@@ -4,9 +4,11 @@ import (
 	"bytes"
 	"encoding/binary"
 	"errors"
+	"math"
 	"os"
 	"strings"
 	"testing"
+	"unsafe"
 )
 
 func testAllocateStringBuffer(t *testing.T, str string) []byte {
@@ -58,7 +60,7 @@ func TestStringRoundTripTemp(t *testing.T) {
 	reader := bytes.NewReader(buf)
 	binary.Read(reader, binary.LittleEndian, &fileNameLen)
 	reader.Seek(int64(BUFFER_HEADER_SIZE), 0)
-	fileName := string(buf[BUFFER_HEADER_SIZE:BUFFER_HEADER_SIZE-fileNameLen])
+	fileName := string(buf[BUFFER_HEADER_SIZE : BUFFER_HEADER_SIZE-fileNameLen])
 
 	output, result := BufferToStringSafe(&buf)
 	if result != 0 {
@@ -448,3 +450,139 @@ func TestDisableTempFile(t *testing.T) {
 		t.Error("Expected ERR_BUFFER_TOO_SMALL")
 	}
 }
+
+// --- Fuzz targets ---
+//
+// These target the Cobhan FFI boundary: buffers and length headers that, in
+// production, arrive from a foreign caller (C, Java, Python, Node, .NET...)
+// and cannot be trusted to be well-formed. Run with -race: several of the
+// bugs these are designed to catch only manifest under checkptr
+// instrumentation, which plain `go test` does not enable.
+//
+//   go test -race -fuzz=FuzzStringRoundTrip -fuzztime=60s
+
+func FuzzStringRoundTrip(f *testing.F) {
+	f.Add("")
+	f.Add("hello")
+	f.Add("world of strings")
+	f.Fuzz(func(t *testing.T, s string) {
+		buf, result := AllocateStringBuffer(s)
+		if result != ERR_NONE {
+			return
+		}
+		out, result := BufferToStringSafe(&buf)
+		if result != ERR_NONE {
+			t.Fatalf("BufferToStringSafe returned %v for input %q", result, s)
+		}
+		if out != s {
+			t.Fatalf("roundtrip mismatch: got %q want %q", out, s)
+		}
+	})
+}
+
+func FuzzBytesRoundTrip(f *testing.F) {
+	f.Add([]byte{})
+	f.Add([]byte{1, 2, 3, 4})
+	f.Fuzz(func(t *testing.T, b []byte) {
+		buf, result := AllocateBytesBuffer(b)
+		if result != ERR_NONE {
+			return
+		}
+		out, result := BufferToBytesSafe(&buf)
+		if result != ERR_NONE {
+			t.Fatalf("BufferToBytesSafe returned %v for input %v", result, b)
+		}
+		if !bytes.Equal(out, b) {
+			t.Fatalf("roundtrip mismatch: got %v want %v", out, b)
+		}
+	})
+}
+
+func FuzzBufferToJson(f *testing.F) {
+	f.Add([]byte(testJson))
+	f.Add([]byte("[1,2,3]"))
+	f.Add([]byte(`"just a string"`))
+	f.Add([]byte("42"))
+	f.Add([]byte("null"))
+	f.Add([]byte("true"))
+	f.Fuzz(func(t *testing.T, data []byte) {
+		buf, result := AllocateBytesBuffer(data)
+		if result != ERR_NONE {
+			return
+		}
+		// Must never panic, regardless of whether data is valid JSON, valid
+		// JSON that isn't an object, or garbage -- only ERR_NONE or
+		// ERR_JSON_DECODE_FAILED are acceptable outcomes.
+		_, result = BufferToJsonSafe(&buf)
+		if result != ERR_NONE && result != ERR_JSON_DECODE_FAILED {
+			t.Fatalf("BufferToJsonSafe returned unexpected error %v for input %q", result, data)
+		}
+	})
+}
+
+func FuzzBufferToJsonStruct(f *testing.F) {
+	f.Add([]byte(testJson))
+	f.Add([]byte("[1,2,3]"))
+	f.Add([]byte(`"just a string"`))
+	f.Fuzz(func(t *testing.T, data []byte) {
+		buf, result := AllocateBytesBuffer(data)
+		if result != ERR_NONE {
+			return
+		}
+		dst := &MyStruct{}
+		// Must never panic; ERR_NONE or ERR_JSON_DECODE_FAILED are the only
+		// acceptable outcomes.
+		result = BufferToJsonStructSafe(&buf, dst)
+		if result != ERR_NONE && result != ERR_JSON_DECODE_FAILED {
+			t.Fatalf("BufferToJsonStructSafe returned unexpected error %v for input %q", result, data)
+		}
+	})
+}
+
+// FuzzRawBufferHeader constructs a raw Cobhan buffer by hand -- an arbitrary
+// length header followed by arbitrary payload bytes -- bypassing
+// AllocateBuffer entirely. This simulates a foreign caller handing Go a
+// buffer with a header that doesn't match the actual data it allocated,
+// which is exactly the threat model the ERR_BUFFER_TOO_LARGE/ERR_BUFFER_TOO_SMALL
+// error codes exist for.
+//
+// A header claiming more data than the payload actually holds -- whether
+// directly (a non-negative header larger than len(payload)) or via the
+// temp-file-name-length encoding (a negative header whose negation is
+// larger than len(payload)) -- is deliberately excluded (via t.Skip
+// below), not exercised here: there is no independent capacity available
+// to validate the header against in bufferPtrToBytes/bufferPtrToString, so
+// a mismatch in that direction is an inherent, unfixable limitation of the
+// wire format rather than something this target can usefully assert on --
+// see the comment on bufferPtrToBytes. math.MinInt32 is the one exception:
+// its negation overflows back to itself, which tempToBytes explicitly
+// guards against, so it's always safe regardless of payload length and
+// must not be skipped (it's the regression case for that fix).
+func FuzzRawBufferHeader(f *testing.F) {
+	f.Add(int32(0), []byte{})                       // well-formed empty buffer
+	f.Add(int32(4), []byte{1, 2, 3, 4})             // well-formed buffer
+	f.Add(int32(-1), []byte{1, 2, 3, 4})            // temp-file path, garbage filename
+	f.Add(int32(math.MinInt32), []byte{1, 2, 3, 4}) // negation overflow in tempToBytes
+	f.Fuzz(func(t *testing.T, header int32, payload []byte) {
+		if header != math.MinInt32 {
+			claimedLen := int64(header)
+			if claimedLen < 0 {
+				claimedLen = -claimedLen
+			}
+			if claimedLen > int64(len(payload)) {
+				t.Skip("header claiming more data than the buffer actually holds is an accepted, unfixable limitation -- see bufferPtrToBytes")
+			}
+		}
+
+		buf := make([]byte, BUFFER_HEADER_SIZE+len(payload))
+		*(*int32)(unsafe.Pointer(&buf[0])) = header
+		copy(buf[BUFFER_HEADER_SIZE:], payload)
+
+		ptr := unsafe.Pointer(&buf[0])
+
+		// Must never panic or fatally crash the process for any header
+		// that's within the range this buffer can actually satisfy.
+		BufferToBytes(ptr)
+		BufferToString(ptr)
+	})
+}

scripts/test.sh

@@ -1,2 +1,2 @@
 #!/bin/bash
-go test -v -failfast -coverprofile cover.out
+go test -race -v -failfast -coverprofile cover.out

testdata/fuzz/FuzzBufferToJson/caf81e9797b19c76

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("")

testdata/fuzz/FuzzBufferToJsonStruct/caf81e9797b19c76

@@ -0,0 +1,2 @@
+go test fuzz v1
+[]byte("")