From: Joe Tsai <joetsai@digital-static.net>
Date: Wed, 24 Aug 2022 02:10:45 +0000 (-0700)
Subject: reflect: fix Value.SetIterXXX to check for the read-only bit
X-Git-Tag: go1.20rc1~1305
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=91ef076562dfcf783074dbd84ad7c6db60fdd481;p=gostls13.git

reflect: fix Value.SetIterXXX to check for the read-only bit

v.SetIterXXX(i) is semantically identical to v.Set(i.XXX()).
If the latter panics for unexported values, so should the former.

This change may breaking some programs, but the change is justified
under the "Go 1 and the Future of Go Programs" document because
the "library has a bug that violates the specification".
In this case, the "reflect" package does not accurately match
the behavior of the Go language specification.
Also, this API was recently released, so the number of users
who could be depending on this behavior is hopefully lower.

Fixes #54628

Change-Id: If86ede51f286e38093f6697944c089f616525115
Reviewed-on: https://go-review.googlesource.com/c/go/+/425184
Auto-Submit: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: David Chase <drchase@google.com>
---

diff --git a/src/reflect/all_test.go b/src/reflect/all_test.go
index 69d5378049..6cc1c9abad 100644
--- a/src/reflect/all_test.go
+++ b/src/reflect/all_test.go
@@ -8001,6 +8001,17 @@ func TestSetIter(t *testing.T) {
 	if got := *y.Interface().(*int); got != b {
 		t.Errorf("pointer incorrect: got %d want %d", got, b)
 	}
+
+	// Make sure we panic assigning from an unexported field.
+	m = ValueOf(struct{ m map[string]int }{data}).Field(0)
+	for iter := m.MapRange(); iter.Next(); {
+		shouldPanic("using value obtained using unexported field", func() {
+			k.SetIterKey(iter)
+		})
+		shouldPanic("using value obtained using unexported field", func() {
+			v.SetIterValue(iter)
+		})
+	}
 }
 
 func TestMethodCallValueCodePtr(t *testing.T) {
diff --git a/src/reflect/value.go b/src/reflect/value.go
index 2589a6dd18..5276af6fd6 100644
--- a/src/reflect/value.go
+++ b/src/reflect/value.go
@@ -1837,7 +1837,8 @@ func (iter *MapIter) Key() Value {
 
 // SetIterKey assigns to v the key of iter's current map entry.
 // It is equivalent to v.Set(iter.Key()), but it avoids allocating a new Value.
-// As in Go, the key must be assignable to v's type.
+// As in Go, the key must be assignable to v's type and
+// must not be derived from an unexported field.
 func (v Value) SetIterKey(iter *MapIter) {
 	if !iter.hiter.initialized() {
 		panic("reflect: Value.SetIterKey called before Next")
@@ -1856,6 +1857,7 @@ func (v Value) SetIterKey(iter *MapIter) {
 	t := (*mapType)(unsafe.Pointer(iter.m.typ))
 	ktype := t.key
 
+	iter.m.mustBeExported() // do not let unexported m leak
 	key := Value{ktype, iterkey, iter.m.flag | flag(ktype.Kind()) | flagIndir}
 	key = key.assignTo("reflect.MapIter.SetKey", v.typ, target)
 	typedmemmove(v.typ, v.ptr, key.ptr)
@@ -1878,7 +1880,8 @@ func (iter *MapIter) Value() Value {
 
 // SetIterValue assigns to v the value of iter's current map entry.
 // It is equivalent to v.Set(iter.Value()), but it avoids allocating a new Value.
-// As in Go, the value must be assignable to v's type.
+// As in Go, the value must be assignable to v's type and
+// must not be derived from an unexported field.
 func (v Value) SetIterValue(iter *MapIter) {
 	if !iter.hiter.initialized() {
 		panic("reflect: Value.SetIterValue called before Next")
@@ -1897,6 +1900,7 @@ func (v Value) SetIterValue(iter *MapIter) {
 	t := (*mapType)(unsafe.Pointer(iter.m.typ))
 	vtype := t.elem
 
+	iter.m.mustBeExported() // do not let unexported m leak
 	elem := Value{vtype, iterelem, iter.m.flag | flag(vtype.Kind()) | flagIndir}
 	elem = elem.assignTo("reflect.MapIter.SetValue", v.typ, target)
 	typedmemmove(v.typ, v.ptr, elem.ptr)
@@ -2218,7 +2222,8 @@ func (v Value) send(x Value, nb bool) (selected bool) {
 
 // Set assigns x to the value v.
 // It panics if CanSet returns false.
-// As in Go, x's value must be assignable to v's type.
+// As in Go, x's value must be assignable to v's type and
+// must not be derived from an unexported field.
 func (v Value) Set(x Value) {
 	v.mustBeAssignable()
 	x.mustBeExported() // do not let unexported x leak