From 468dcc605a6742099e86edd96c4d5eaa0890d552 Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 5 Jun 2026 15:05:26 +0900 Subject: [PATCH 1/5] fix(ffitemplate): clone input slices where necessary --- ffitemplate/_tmpl/driver.go.tmpl | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/ffitemplate/_tmpl/driver.go.tmpl b/ffitemplate/_tmpl/driver.go.tmpl index e0a72bd..28f760e 100644 --- a/ffitemplate/_tmpl/driver.go.tmpl +++ b/ffitemplate/_tmpl/driver.go.tmpl @@ -60,6 +60,7 @@ import ( "os" "runtime" "runtime/cgo" + "slices" "strings" "sync/atomic" "unsafe" @@ -148,6 +149,7 @@ func setErrWithDetails(err *C.struct_AdbcError, adbcError adbc.Error) { cErr.values = (**C.cuint8_t)(C.calloc(C.size_t(numDetails), C.size_t(unsafe.Sizeof((*C.cuint8_t)(nil))))) cErr.lengths = (*C.size_t)(C.calloc(C.size_t(numDetails), C.sizeof_size_t)) + // SAFETY: no copy of fromCArr because these are written to, not read from keys := fromCArr[*C.cchar_t](cErr.keys, numDetails) values := fromCArr[*C.cuint8_t](cErr.values, numDetails) lengths := fromCArr[C.size_t](cErr.lengths, numDetails) @@ -266,6 +268,7 @@ func getFromHandle[T any](ptr unsafe.Pointer) *T { func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusCode { lenWithTerminator := C.size_t(len(val) + 1) if lenWithTerminator <= *length { + // SAFETY: no copy of fromCArr because this is written to, not read from sink := fromCArr[byte]((*byte)(unsafe.Pointer(out)), int(*length)) copy(sink, val) sink[len(val)] = 0 @@ -276,6 +279,7 @@ func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusC func exportBytesOption(val []byte, out *C.uint8_t, length *C.size_t) C.AdbcStatusCode { if C.size_t(len(val)) <= *length { + // SAFETY: no copy of fromCArr because this is written to, not read from sink := fromCArr[byte]((*byte)(out), int(*length)) copy(sink, val) } @@ -703,7 +707,7 @@ func {{.Prefix}}DatabaseSetOptionBytes(db *C.struct_AdbcDatabase, key *C.cchar_t } cdb := getFromHandle[cDatabase](db.private_data) k := C.GoString(key) - v := fromCArr[byte](value, int(length)) + v := slices.Clone(fromCArr[byte](value, int(length))) if cdb.db != nil { e := cdb.db.SetOptionBytes(cdb.newContext(), k, v) @@ -924,7 +928,7 @@ func {{.Prefix}}ConnectionSetOptionBytes(db *C.struct_AdbcConnection, key *C.cch return C.ADBC_STATUS_INVALID_STATE } - e := conn.cnxn.SetOptionBytes(conn.newContext(), C.GoString(key), fromCArr[byte](value, int(length))) + e := conn.cnxn.SetOptionBytes(conn.newContext(), C.GoString(key), slices.Clone(fromCArr[byte](value, int(length)))) return C.AdbcStatusCode(errToAdbcErr(err, e)) } @@ -1034,6 +1038,7 @@ func {{.Prefix}}ConnectionRelease(cnxn *C.struct_AdbcConnection, err *C.struct_A return C.AdbcStatusCode(errToAdbcErr(err, conn.cnxn.Close(conn.newContext()))) } +// SAFETY: at each call site, consider whether a copy of the resulting slice must be made func fromCArr[T, CType any](ptr *CType, sz int) []T { if ptr == nil || sz == 0 { return nil @@ -1106,7 +1111,7 @@ func {{.Prefix}}ConnectionGetInfo(cnxn *C.struct_AdbcConnection, codes *C.cuint3 return C.ADBC_STATUS_INVALID_STATE } - infoCodes := fromCArr[adbc.InfoCode](codes, int(len)) + infoCodes := slices.Clone(fromCArr[adbc.InfoCode](codes, int(len))) rdr, e := conn.cnxn.GetInfo(conn.newContext(), infoCodes) if e != nil { return C.AdbcStatusCode(errToAdbcErr(err, e)) @@ -1247,7 +1252,7 @@ func {{.Prefix}}ConnectionReadPartition(cnxn *C.struct_AdbcConnection, serialize return C.ADBC_STATUS_INVALID_STATE } - rdr, e := conn.cnxn.ReadPartition(conn.newContext(), fromCArr[byte](serialized, int(serializedLen))) + rdr, e := conn.cnxn.ReadPartition(conn.newContext(), slices.Clone(fromCArr[byte](serialized, int(serializedLen)))) if e != nil { return C.AdbcStatusCode(errToAdbcErr(err, e)) } @@ -1605,7 +1610,7 @@ func {{.Prefix}}StatementSetSubstraitPlan(stmt *C.struct_AdbcStatement, plan *C. return C.ADBC_STATUS_INVALID_STATE } - e := st.stmt.SetSubstraitPlan(st.newContext(), fromCArr[byte](plan, int(length))) + e := st.stmt.SetSubstraitPlan(st.newContext(), slices.Clone(fromCArr[byte](plan, int(length)))) return C.AdbcStatusCode(errToAdbcErr(err, e)) } @@ -1706,7 +1711,7 @@ func {{.Prefix}}StatementSetOptionBytes(db *C.struct_AdbcStatement, key *C.cchar return C.ADBC_STATUS_NOT_IMPLEMENTED } - e := opts.SetOptionBytes(st.newContext(), C.GoString(key), fromCArr[byte](value, int(length))) + e := opts.SetOptionBytes(st.newContext(), C.GoString(key), slices.Clone(fromCArr[byte](value, int(length)))) return C.AdbcStatusCode(errToAdbcErr(err, e)) } @@ -1808,6 +1813,7 @@ func {{.Prefix}}StatementExecutePartitions(stmt *C.struct_AdbcStatement, schema totalLen += len(p) } partitions.private_data = C.calloc(C.size_t(totalLen), C.size_t(1)) + // SAFETY: no copy of fromCArr because this is written to, not read from dst := fromCArr[byte]((*byte)(partitions.private_data), totalLen) partIDs := fromCArr[*C.cuint8_t](partitions.partitions, int(partitions.num_partitions)) @@ -1829,9 +1835,11 @@ func AdbcDriver{{.Prefix}}Init(version C.int, rawDriver *C.void, err *C.struct_A switch version { case C.ADBC_VERSION_1_0_0: + // SAFETY: no copy of fromCArr because this is written to, not read from sink := fromCArr[byte]((*byte)(unsafe.Pointer(driver)), C.ADBC_DRIVER_1_0_0_SIZE) memory.Set(sink, 0) case C.ADBC_VERSION_1_1_0: + // SAFETY: no copy of fromCArr because this is written to, not read from sink := fromCArr[byte]((*byte)(unsafe.Pointer(driver)), C.ADBC_DRIVER_1_1_0_SIZE) memory.Set(sink, 0) default: From 117436d6f80f13193f4ee68f9740d78da1f6b12f Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 5 Jun 2026 15:34:40 +0900 Subject: [PATCH 2/5] also check int cast --- ffitemplate/_tmpl/driver.go.tmpl | 39 ++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/ffitemplate/_tmpl/driver.go.tmpl b/ffitemplate/_tmpl/driver.go.tmpl index 28f760e..fbac0b2 100644 --- a/ffitemplate/_tmpl/driver.go.tmpl +++ b/ffitemplate/_tmpl/driver.go.tmpl @@ -707,7 +707,11 @@ func {{.Prefix}}DatabaseSetOptionBytes(db *C.struct_AdbcDatabase, key *C.cchar_t } cdb := getFromHandle[cDatabase](db.private_data) k := C.GoString(key) - v := slices.Clone(fromCArr[byte](value, int(length))) + var safeLen int + if safeLen, code = checkLengthToInt(length, err); code != C.ADBC_STATUS_OK { + return code + } + v := slices.Clone(fromCArr[byte](value, safeLen)) if cdb.db != nil { e := cdb.db.SetOptionBytes(cdb.newContext(), k, v) @@ -928,7 +932,11 @@ func {{.Prefix}}ConnectionSetOptionBytes(db *C.struct_AdbcConnection, key *C.cch return C.ADBC_STATUS_INVALID_STATE } - e := conn.cnxn.SetOptionBytes(conn.newContext(), C.GoString(key), slices.Clone(fromCArr[byte](value, int(length)))) + var safeLen int + if safeLen, code = checkLengthToInt(length, err); code != C.ADBC_STATUS_OK { + return code + } + e := conn.cnxn.SetOptionBytes(conn.newContext(), C.GoString(key), slices.Clone(fromCArr[byte](value, safeLen))) return C.AdbcStatusCode(errToAdbcErr(err, e)) } @@ -1047,6 +1055,14 @@ func fromCArr[T, CType any](ptr *CType, sz int) []T { return unsafe.Slice((*T)(unsafe.Pointer(ptr)), sz) } +func checkLengthToInt(length C.size_t, err *C.struct_AdbcError) (int, C.AdbcStatusCode) { + if length > C.size_t(math.MaxInt) { + setErr(err, "Length %d exceeds max Go int %d", length, math.MaxInt) + return 0, C.ADBC_STATUS_INVALID_ARGUMENT + } + return int(length), 0 +} + func toCdataStream(ptr *C.struct_ArrowArrayStream) *cdata.CArrowArrayStream { return (*cdata.CArrowArrayStream)(unsafe.Pointer(ptr)) } @@ -1111,7 +1127,11 @@ func {{.Prefix}}ConnectionGetInfo(cnxn *C.struct_AdbcConnection, codes *C.cuint3 return C.ADBC_STATUS_INVALID_STATE } - infoCodes := slices.Clone(fromCArr[adbc.InfoCode](codes, int(len))) + var safeLen int + if safeLen, code = checkLengthToInt(len, err); code != C.ADBC_STATUS_OK { + return code + } + infoCodes := slices.Clone(fromCArr[adbc.InfoCode](codes, safeLen)) rdr, e := conn.cnxn.GetInfo(conn.newContext(), infoCodes) if e != nil { return C.AdbcStatusCode(errToAdbcErr(err, e)) @@ -1610,7 +1630,11 @@ func {{.Prefix}}StatementSetSubstraitPlan(stmt *C.struct_AdbcStatement, plan *C. return C.ADBC_STATUS_INVALID_STATE } - e := st.stmt.SetSubstraitPlan(st.newContext(), slices.Clone(fromCArr[byte](plan, int(length)))) + var safeLen int + if safeLen, code = checkLengthToInt(length, err); code != C.ADBC_STATUS_OK { + return code + } + e := st.stmt.SetSubstraitPlan(st.newContext(), slices.Clone(fromCArr[byte](plan, safeLen))) return C.AdbcStatusCode(errToAdbcErr(err, e)) } @@ -1711,7 +1735,12 @@ func {{.Prefix}}StatementSetOptionBytes(db *C.struct_AdbcStatement, key *C.cchar return C.ADBC_STATUS_NOT_IMPLEMENTED } - e := opts.SetOptionBytes(st.newContext(), C.GoString(key), slices.Clone(fromCArr[byte](value, int(length)))) + + var safeLen int + if safeLen, code = checkLengthToInt(length, err); code != C.ADBC_STATUS_OK { + return code + } + e := opts.SetOptionBytes(st.newContext(), C.GoString(key), slices.Clone(fromCArr[byte](value, safeLen))) return C.AdbcStatusCode(errToAdbcErr(err, e)) } From 1b33dc11f57e1dbc26a76ccf8851442440f15589 Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 5 Jun 2026 15:38:13 +0900 Subject: [PATCH 3/5] feedback --- ffitemplate/_tmpl/driver.go.tmpl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ffitemplate/_tmpl/driver.go.tmpl b/ffitemplate/_tmpl/driver.go.tmpl index fbac0b2..d9a50c8 100644 --- a/ffitemplate/_tmpl/driver.go.tmpl +++ b/ffitemplate/_tmpl/driver.go.tmpl @@ -57,6 +57,7 @@ import ( "fmt" "io" "log/slog" + "math" "os" "runtime" "runtime/cgo" @@ -1060,7 +1061,7 @@ func checkLengthToInt(length C.size_t, err *C.struct_AdbcError) (int, C.AdbcStat setErr(err, "Length %d exceeds max Go int %d", length, math.MaxInt) return 0, C.ADBC_STATUS_INVALID_ARGUMENT } - return int(length), 0 + return int(length), C.ADBC_STATUS_OK } func toCdataStream(ptr *C.struct_ArrowArrayStream) *cdata.CArrowArrayStream { From 44cd5da992a959a7536f535610b35b050c839682 Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 5 Jun 2026 15:38:59 +0900 Subject: [PATCH 4/5] feedback --- ffitemplate/_tmpl/driver.go.tmpl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/ffitemplate/_tmpl/driver.go.tmpl b/ffitemplate/_tmpl/driver.go.tmpl index d9a50c8..c8bc31e 100644 --- a/ffitemplate/_tmpl/driver.go.tmpl +++ b/ffitemplate/_tmpl/driver.go.tmpl @@ -1273,7 +1273,11 @@ func {{.Prefix}}ConnectionReadPartition(cnxn *C.struct_AdbcConnection, serialize return C.ADBC_STATUS_INVALID_STATE } - rdr, e := conn.cnxn.ReadPartition(conn.newContext(), slices.Clone(fromCArr[byte](serialized, int(serializedLen)))) + var safeLen int + if safeLen, code = checkLengthToInt(serializedLen, err); code != C.ADBC_STATUS_OK { + return code + } + rdr, e := conn.cnxn.ReadPartition(conn.newContext(), slices.Clone(fromCArr[byte](serialized, safeLen))) if e != nil { return C.AdbcStatusCode(errToAdbcErr(err, e)) } From 1eafd262600e90736ab472de4a9acf4a6c4897aa Mon Sep 17 00:00:00 2001 From: David Li Date: Fri, 5 Jun 2026 15:41:39 +0900 Subject: [PATCH 5/5] feedback --- ffitemplate/_tmpl/driver.go.tmpl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ffitemplate/_tmpl/driver.go.tmpl b/ffitemplate/_tmpl/driver.go.tmpl index c8bc31e..e591341 100644 --- a/ffitemplate/_tmpl/driver.go.tmpl +++ b/ffitemplate/_tmpl/driver.go.tmpl @@ -267,10 +267,10 @@ func getFromHandle[T any](ptr unsafe.Pointer) *T { } func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusCode { - lenWithTerminator := C.size_t(len(val) + 1) + lenWithTerminator := C.size_t(len(val)+1) if lenWithTerminator <= *length { // SAFETY: no copy of fromCArr because this is written to, not read from - sink := fromCArr[byte]((*byte)(unsafe.Pointer(out)), int(*length)) + sink := fromCArr[byte]((*byte)(unsafe.Pointer(out)), len(val)+1) copy(sink, val) sink[len(val)] = 0 } @@ -281,7 +281,7 @@ func exportStringOption(val string, out *C.char, length *C.size_t) C.AdbcStatusC func exportBytesOption(val []byte, out *C.uint8_t, length *C.size_t) C.AdbcStatusCode { if C.size_t(len(val)) <= *length { // SAFETY: no copy of fromCArr because this is written to, not read from - sink := fromCArr[byte]((*byte)(out), int(*length)) + sink := fromCArr[byte]((*byte)(out), len(val)) copy(sink, val) } *length = C.size_t(len(val))