1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
|
From c23579f031ecd09bf37c644723b33736dffa8b92 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Tue, 23 Jan 2024 15:59:47 -0800
Subject: [PATCH] database/sql: avoid clobbering driver-owned memory in
RawBytes
Depending on the query, a RawBytes can contain memory owned by the
driver or by database/sql:
If the driver provides the column as a []byte,
RawBytes aliases that []byte.
If the driver provides the column as any other type,
RawBytes contains memory allocated by database/sql.
Prior to this CL, Rows.Scan will reuse existing capacity in a
RawBytes to permit a single allocation to be reused across rows.
When a RawBytes is reused across queries, this can result
in database/sql writing to driver-owned memory.
Add a buffer to Rows to store RawBytes data, and reuse this
buffer across calls to Rows.Scan.
Fixes #65201
Change-Id: Iac640174c7afa97eeb39496f47dec202501b2483
Reviewed-on: https://go-review.googlesource.com/c/go/+/557917
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
CVE: CVE-2025-47907
Upstream-Status: Backport [https://github.com/golang/go/commit/c23579f031ecd09bf37c644723b33736dffa8b92]
Signed-off-by: Praveen Kumar <praveen.kumar@windriver.com>
---
src/database/sql/convert.go | 8 +++---
src/database/sql/convert_test.go | 14 +++++++---
src/database/sql/sql.go | 34 +++++++++++++++++++++++
src/database/sql/sql_test.go | 47 ++++++++++++++++++++++++++++++++
4 files changed, 95 insertions(+), 8 deletions(-)
diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index b966ef9..3a581f6 100644
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -237,7 +237,7 @@ func convertAssignRows(dest, src interface{}, rows *Rows) error {
if d == nil {
return errNilPtr
}
- *d = append((*d)[:0], s...)
+ *d = rows.setrawbuf(append(rows.rawbuf(), s...))
return nil
}
case []byte:
@@ -285,7 +285,7 @@ func convertAssignRows(dest, src interface{}, rows *Rows) error {
if d == nil {
return errNilPtr
}
- *d = s.AppendFormat((*d)[:0], time.RFC3339Nano)
+ *d = rows.setrawbuf(s.AppendFormat(rows.rawbuf(), time.RFC3339Nano))
return nil
}
case decimalDecompose:
@@ -366,8 +366,8 @@ func convertAssignRows(dest, src interface{}, rows *Rows) error {
}
case *RawBytes:
sv = reflect.ValueOf(src)
- if b, ok := asBytes([]byte(*d)[:0], sv); ok {
- *d = RawBytes(b)
+ if b, ok := asBytes(rows.rawbuf(), sv); ok {
+ *d = rows.setrawbuf(b)
return nil
}
case *bool:
diff --git a/src/database/sql/convert_test.go b/src/database/sql/convert_test.go
index 2668a5e..23a70bf 100644
--- a/src/database/sql/convert_test.go
+++ b/src/database/sql/convert_test.go
@@ -357,9 +357,10 @@ func TestRawBytesAllocs(t *testing.T) {
{"time", time.Unix(2, 5).UTC(), "1970-01-01T00:00:02.000000005Z"},
}
- buf := make(RawBytes, 10)
- test := func(name string, in interface{}, want string) {
- if err := convertAssign(&buf, in); err != nil {
+ var buf RawBytes
+ rows := &Rows{}
+ test := func(name string, in interface{}, want string) {
+ if err := convertAssignRows(&buf, in, rows); err != nil {
t.Fatalf("%s: convertAssign = %v", name, err)
}
match := len(buf) == len(want)
@@ -378,6 +379,7 @@ func TestRawBytesAllocs(t *testing.T) {
n := testing.AllocsPerRun(100, func() {
for _, tt := range tests {
+ rows.raw = rows.raw[:0]
test(tt.name, tt.in, tt.want)
}
})
@@ -386,7 +388,11 @@ func TestRawBytesAllocs(t *testing.T) {
// and gc. With 32-bit words there are more convT2E allocs, and
// with gccgo, only pointers currently go in interface data.
// So only care on amd64 gc for now.
- measureAllocs := runtime.GOARCH == "amd64" && runtime.Compiler == "gc"
+ measureAllocs := false
+ switch runtime.GOARCH {
+ case "amd64", "arm64":
+ measureAllocs = runtime.Compiler == "gc"
+ }
if n > 0.5 && measureAllocs {
t.Fatalf("allocs = %v; want 0", n)
diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index ef49e70..e25447c 100644
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -2894,6 +2894,13 @@ type Rows struct {
// not to be called concurrently.
lastcols []driver.Value
+ // raw is a buffer for RawBytes that persists between Scan calls.
+ // This is used when the driver returns a mismatched type that requires
+ // a cloning allocation. For example, if the driver returns a *string and
+ // the user is scanning into a *RawBytes, we need to copy the string.
+ // The raw buffer here lets us reuse the memory for that copy across Scan calls.
+ raw []byte
+
// closemuScanHold is whether the previous call to Scan kept closemu RLock'ed
// without unlocking it. It does that when the user passes a *RawBytes scan
// target. In that case, we need to prevent awaitDone from closing the Rows
@@ -3068,6 +3075,32 @@ func (rs *Rows) Err() error {
return rs.lasterrOrErrLocked(nil)
}
+// rawbuf returns the buffer to append RawBytes values to.
+// This buffer is reused across calls to Rows.Scan.
+//
+// Usage:
+//
+// rawBytes = rows.setrawbuf(append(rows.rawbuf(), value...))
+func (rs *Rows) rawbuf() []byte {
+ if rs == nil {
+ // convertAssignRows can take a nil *Rows; for simplicity handle it here
+ return nil
+ }
+ return rs.raw
+}
+
+// setrawbuf updates the RawBytes buffer with the result of appending a new value to it.
+// It returns the new value.
+func (rs *Rows) setrawbuf(b []byte) RawBytes {
+ if rs == nil {
+ // convertAssignRows can take a nil *Rows; for simplicity handle it here
+ return RawBytes(b)
+ }
+ off := len(rs.raw)
+ rs.raw = b
+ return RawBytes(rs.raw[off:])
+}
+
var errRowsClosed = errors.New("sql: Rows are closed")
var errNoRows = errors.New("sql: no Rows available")
@@ -3275,6 +3308,7 @@ func (rs *Rows) Scan(dest ...interface{}) error {
if scanArgsContainRawBytes(dest) {
rs.closemuScanHold = true
+ rs.raw = rs.raw[:0]
} else {
rs.closemu.RUnlock()
}
diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 53b38d1..6aa9bf0 100644
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -4313,6 +4313,53 @@ func TestContextCancelDuringRawBytesScan(t *testing.T) {
}
}
+// Issue #65201.
+//
+// If a RawBytes is reused across multiple queries,
+// subsequent queries shouldn't overwrite driver-owned memory from previous queries.
+func TestRawBytesReuse(t *testing.T) {
+ db := newTestDB(t, "people")
+ defer closeDB(t, db)
+
+ if _, err := db.Exec("USE_RAWBYTES"); err != nil {
+ t.Fatal(err)
+ }
+
+ var raw RawBytes
+
+ // The RawBytes in this query aliases driver-owned memory.
+ rows, err := db.Query("SELECT|people|name|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ rows.Next()
+ rows.Scan(&raw) // now raw is pointing to driver-owned memory
+ name1 := string(raw)
+ rows.Close()
+
+ // The RawBytes in this query does not alias driver-owned memory.
+ rows, err = db.Query("SELECT|people|age|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ rows.Next()
+ rows.Scan(&raw) // this must not write to the driver-owned memory in raw
+ rows.Close()
+
+ // Repeat the first query. Nothing should have changed.
+ rows, err = db.Query("SELECT|people|name|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ rows.Next()
+ rows.Scan(&raw) // raw points to driver-owned memory again
+ name2 := string(raw)
+ rows.Close()
+ if name1 != name2 {
+ t.Fatalf("Scan read name %q, want %q", name2, name1)
+ }
+}
+
// badConn implements a bad driver.Conn, for TestBadDriver.
// The Exec method panics.
type badConn struct{}
|
