summaryrefslogtreecommitdiffstats
path: root/meta/recipes-devtools/go/go-1.21/CVE-2025-47907.patch
blob: b2af7df81bfca860204dd864c25ad9be9d706a1a (plain)
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
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
From 8a924caaf348fdc366bab906424616b2974ad4e9 Mon Sep 17 00:00:00 2001
From: Damien Neil <dneil@google.com>
Date: Wed, 23 Jul 2025 14:26:54 -0700
Subject: [PATCH] database/sql: avoid closing Rows while scan is in progress

A database/sql/driver.Rows can return database-owned data
from Rows.Next. The driver.Rows documentation doesn't explicitly
document the lifetime guarantees for this data, but a reasonable
expectation is that the caller of Next should only access it
until the next call to Rows.Close or Rows.Next.

Avoid violating that constraint when a query is cancelled while
a call to database/sql.Rows.Scan (note the difference between
the two different Rows types!) is in progress. We previously
took care to avoid closing a driver.Rows while the user has
access to driver-owned memory via a RawData, but we could still
close a driver.Rows while a Scan call was in the process of
reading previously-returned driver-owned data.

Update the fake DB used in database/sql tests to invalidate
returned data to help catch other places we might be
incorrectly retaining it.

Updates #74831
Fixes #74832

Change-Id: Ice45b5fad51b679c38e3e1d21ef39156b56d6037
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2540
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Reviewed-on: https://go-internal-review.googlesource.com/c/go/+/2601
Reviewed-on: https://go-review.googlesource.com/c/go/+/693558
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Mark Freeman <markfreeman@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>

CVE: CVE-2025-47907

Upstream-Status: Backport [https://github.com/golang/go/commit/8a924caaf348fdc366bab906424616b2974ad4e9]

Signed-off-by: Praveen Kumar <praveen.kumar@windriver.com>
---
 src/database/sql/convert.go     |  2 --
 src/database/sql/fakedb_test.go | 47 ++++++++++++--------------
 src/database/sql/sql.go         | 26 +++++++-------
 src/database/sql/sql_test.go    | 60 ++++++++++++++++++++++++++++++---
 4 files changed, 90 insertions(+), 45 deletions(-)

diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index 3a581f6..5b0c6f0 100644
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -324,7 +324,6 @@ func convertAssignRows(dest, src interface{}, rows *Rows) error {
			if rows == nil {
				return errors.New("invalid context to convert cursor rows, missing parent *Rows")
			}
-			rows.closemu.Lock()
			*d = Rows{
				dc:          rows.dc,
				releaseConn: func(error) {},
@@ -340,7 +339,6 @@ func convertAssignRows(dest, src interface{}, rows *Rows) error {
					parentCancel()
				}
			}
-			rows.closemu.Unlock()
			return nil
		}
	}
diff --git a/src/database/sql/fakedb_test.go b/src/database/sql/fakedb_test.go
index 33c57b9..9f3d517 100644
--- a/src/database/sql/fakedb_test.go
+++ b/src/database/sql/fakedb_test.go
@@ -5,6 +5,7 @@
 package sql

 import (
+	"bytes"
	"context"
	"database/sql/driver"
	"errors"
@@ -15,7 +16,6 @@ import (
	"strconv"
	"strings"
	"sync"
-	"sync/atomic"
	"testing"
	"time"
 )
@@ -91,8 +91,6 @@ func (cc *fakeDriverCtx) OpenConnector(name string) (driver.Connector, error) {
 type fakeDB struct {
	name string

-	useRawBytes atomic.Bool
-
	mu       sync.Mutex
	tables   map[string]*table
	badConn  bool
@@ -683,8 +681,6 @@ func (c *fakeConn) PrepareContext(ctx context.Context, query string) (driver.Stm
		switch cmd {
		case "WIPE":
			// Nothing
-		case "USE_RAWBYTES":
-			c.db.useRawBytes.Store(true)
		case "SELECT":
			stmt, err = c.prepareSelect(stmt, parts)
		case "CREATE":
@@ -788,9 +784,6 @@ func (s *fakeStmt) ExecContext(ctx context.Context, args []driver.NamedValue) (d
	case "WIPE":
		db.wipe()
		return driver.ResultNoRows, nil
-	case "USE_RAWBYTES":
-		s.c.db.useRawBytes.Store(true)
-		return driver.ResultNoRows, nil
	case "CREATE":
		if err := db.createTable(s.table, s.colName, s.colType); err != nil {
			return nil, err
@@ -1073,10 +1066,9 @@ type rowsCursor struct {
	errPos int
	err    error

-	// a clone of slices to give out to clients, indexed by the
-	// original slice's first byte address.  we clone them
-	// just so we're able to corrupt them on close.
-	bytesClone map[*byte][]byte
+	// Data returned to clients.
+	// We clone and stash it here so it can be invalidated by Close and Next.
+	driverOwnedMemory [][]byte

	// Every operation writes to line to enable the race detector
	// check for data races.
@@ -1090,9 +1082,19 @@ func (rc *rowsCursor) touchMem() {
	rc.line++
 }

+func (rc *rowsCursor) invalidateDriverOwnedMemory() {
+	for _, buf := range rc.driverOwnedMemory {
+		for i := range buf {
+			buf[i] = 'x'
+		}
+	}
+	rc.driverOwnedMemory = nil
+}
+
 func (rc *rowsCursor) Close() error {
	rc.touchMem()
	rc.parentMem.touchMem()
+	rc.invalidateDriverOwnedMemory()
	rc.closed = true
	return nil
 }
@@ -1123,6 +1125,8 @@ func (rc *rowsCursor) Next(dest []driver.Value) error {
	if rc.posRow >= len(rc.rows[rc.posSet]) {
		return io.EOF // per interface spec
	}
+	// Corrupt any previously returned bytes.
+	rc.invalidateDriverOwnedMemory()
	for i, v := range rc.rows[rc.posSet][rc.posRow].cols {
		// TODO(bradfitz): convert to subset types? naah, I
		// think the subset types should only be input to
@@ -1130,20 +1134,13 @@ func (rc *rowsCursor) Next(dest []driver.Value) error {
		// a wider range of types coming out of drivers. all
		// for ease of drivers, and to prevent drivers from
		// messing up conversions or doing them differently.
-		dest[i] = v
-
-		if bs, ok := v.([]byte); ok && !rc.db.useRawBytes.Load() {
-			if rc.bytesClone == nil {
-				rc.bytesClone = make(map[*byte][]byte)
-			}
-			clone, ok := rc.bytesClone[&bs[0]]
-			if !ok {
-				clone = make([]byte, len(bs))
-				copy(clone, bs)
-				rc.bytesClone[&bs[0]] = clone
-			}
-			dest[i] = clone
+		if bs, ok := v.([]byte); ok {
+			// Clone []bytes and stash for later invalidation.
+			bs = bytes.Clone(bs)
+			rc.driverOwnedMemory = append(rc.driverOwnedMemory, bs)
+			v = bs
		}
+		dest[i] = v
	}
	return nil
 }
diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index e25447c..a428e29 100644
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -3294,38 +3294,36 @@ func (rs *Rows) Scan(dest ...interface{}) error {
		// without calling Next.
		return fmt.Errorf("sql: Scan called without calling Next (closemuScanHold)")
	}
+
	rs.closemu.RLock()
+	rs.raw = rs.raw[:0]
+	err := rs.scanLocked(dest...)
+	if err == nil && scanArgsContainRawBytes(dest) {
+		rs.closemuScanHold = true
+	} else {
+		rs.closemu.RUnlock()
+	}
+	return err
+}

+func (rs *Rows) scanLocked(dest ...interface{}) error {
	if rs.lasterr != nil && rs.lasterr != io.EOF {
-		rs.closemu.RUnlock()
		return rs.lasterr
	}
	if rs.closed {
-		err := rs.lasterrOrErrLocked(errRowsClosed)
-		rs.closemu.RUnlock()
-		return err
-	}
-
-	if scanArgsContainRawBytes(dest) {
-		rs.closemuScanHold = true
-		rs.raw = rs.raw[:0]
-	} else {
-		rs.closemu.RUnlock()
+		return rs.lasterrOrErrLocked(errRowsClosed)
	}

	if rs.lastcols == nil {
-		rs.closemuRUnlockIfHeldByScan()
		return errors.New("sql: Scan called without calling Next")
	}
	if len(dest) != len(rs.lastcols) {
-		rs.closemuRUnlockIfHeldByScan()
		return fmt.Errorf("sql: expected %d destination arguments in Scan, not %d", len(rs.lastcols), len(dest))
	}

	for i, sv := range rs.lastcols {
		err := convertAssignRows(dest[i], sv, rs)
		if err != nil {
-			rs.closemuRUnlockIfHeldByScan()
			return fmt.Errorf(`sql: Scan error on column index %d, name %q: %w`, i, rs.rowsi.Columns()[i], err)
		}
	}
diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index 6aa9bf0..6aec7ec 100644
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -5,6 +5,7 @@
 package sql

 import (
+	"bytes"
	"context"
	"database/sql/driver"
	"errors"
@@ -4321,10 +4322,6 @@ 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.
@@ -4469,6 +4466,61 @@ func TestTypedString(t *testing.T) {
	}
 }

+type testScanner struct {
+	scanf func(src any) error
+}
+
+func (ts testScanner) Scan(src any) error { return ts.scanf(src) }
+
+func TestContextCancelDuringScan(t *testing.T) {
+	db := newTestDB(t, "people")
+	defer closeDB(t, db)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	scanStart := make(chan any)
+	scanEnd := make(chan error)
+	scanner := &testScanner{
+		scanf: func(src any) error {
+			scanStart <- src
+			return <-scanEnd
+		},
+	}
+
+	// Start a query, and pause it mid-scan.
+	want := []byte("Alice")
+	r, err := db.QueryContext(ctx, "SELECT|people|name|name=?", string(want))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !r.Next() {
+		t.Fatalf("r.Next() = false, want true")
+	}
+	go func() {
+		r.Scan(scanner)
+	}()
+	got := <-scanStart
+	defer close(scanEnd)
+	gotBytes, ok := got.([]byte)
+	if !ok {
+		t.Fatalf("r.Scan returned %T, want []byte", got)
+	}
+	if !bytes.Equal(gotBytes, want) {
+		t.Fatalf("before cancel: r.Scan returned %q, want %q", gotBytes, want)
+	}
+
+	// Cancel the query.
+	// Sleep to give it a chance to finish canceling.
+	cancel()
+	time.Sleep(10 * time.Millisecond)
+
+	// Cancelling the query should not have changed the result.
+	if !bytes.Equal(gotBytes, want) {
+		t.Fatalf("after cancel: r.Scan result is now %q, want %q", gotBytes, want)
+	}
+}
+
 func BenchmarkConcurrentDBExec(b *testing.B) {
	b.ReportAllocs()
	ct := new(concurrentDBExecTest)