database/sql: clone data for named []byte types

Previously named byte types like json.RawMessage could get dirty
database memory from a call to Scan. These types would activate a
code path that didn't clone the byte data coming from the database
before assigning it. Another thread could then overwrite the byte
array in src, which has unexpected consequences.

Originally reported by Jason Moiron; the patch and test are his
suggestions. Fixes #13905.

Change-Id: Iacfef61cbc9dd51c8fccef9b2b9d9544c77dd0e0
Reviewed-on: https://go-review.googlesource.com/22393
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/database/sql/convert.go b/src/database/sql/convert.go
index 92c3b689c1103c9f93804b1340a1852f8f6310e5..99aed2398e28b099dae92b78589db468549135ac 100644 (file)
--- a/src/database/sql/convert.go
+++ b/src/database/sql/convert.go
@@ -217,7 +217,12 @@ func convertAssign(dest, src interface{}) error {
 
        dv := reflect.Indirect(dpv)
        if sv.IsValid() && sv.Type().AssignableTo(dv.Type()) {
-               dv.Set(sv)
+               switch b := src.(type) {
+               case []byte:
+                       dv.Set(reflect.ValueOf(cloneBytes(b)))
+               default:
+                       dv.Set(sv)
+               }
                return nil
        }
 

diff --git a/src/database/sql/convert_test.go b/src/database/sql/convert_test.go
index 342875e190c99ed12baf1e14d720e8b512e9526b..ab81f2f65a27bc7bd87d03bb32a1f2a765174349 100644 (file)
--- a/src/database/sql/convert_test.go
+++ b/src/database/sql/convert_test.go
@@ -377,3 +377,15 @@ func TestRawBytesAllocs(t *testing.T) {
                t.Fatalf("allocs = %v; want max 1", n)
        }
 }
+
+// https://github.com/golang/go/issues/13905
+func TestUserDefinedBytes(t *testing.T) {
+       type userDefinedBytes []byte
+       var u userDefinedBytes
+       v := []byte("foo")
+
+       convertAssign(&u, v)
+       if &u[0] == &v[0] {
+               t.Fatal("userDefinedBytes got potentially dirty driver memory")
+       }
+}