diff --git a/internal/state/decode.go b/internal/state/decode.go index 135474e..66a8df7 100644 --- a/internal/state/decode.go +++ b/internal/state/decode.go @@ -744,7 +744,7 @@ func (ds *decodeState) decodeObject(ods *objectDecodeState, obj reflect.Value, e Failf("reflect.Value cannot be assigned to %v", obj.Type()) } if _, invalid := x.Type.(nilType); invalid { - if _, empty := x.Value.(nilValue); !empty || x.Addressable { + if _, empty := x.Value.(nilValue); !empty || x.Addressable || x.ReadOnly != 0 { Failf("invalid zero reflect.Value") } obj.SetZero() @@ -765,6 +765,10 @@ func (ds *decodeState) decodeObject(ods *objectDecodeState, obj reflect.Value, e ds.decodeObject(ods, storage, x.Value) value = reflectValueUnaddressable(storage) } + if x.ReadOnly != 0 { + flag := reflect.ValueOf(&value).Elem().FieldByName("flag") + reflectValueRWAddr(flag).Elem().SetUint(flag.Uint() | x.ReadOnly) + } obj.Set(reflect.ValueOf(value)) default: // Should not happen, not propagated as an error. diff --git a/internal/state/decode_unsafe.go b/internal/state/decode_unsafe.go index 13172d2..a8d2312 100644 --- a/internal/state/decode_unsafe.go +++ b/internal/state/decode_unsafe.go @@ -21,6 +21,10 @@ import ( "unsafe" ) +// Go 1.26 reflect/value.go: flagStickyRO and flagEmbedRO have distinct Field +// propagation rules. Keep both bits instead of collapsing them to CanInterface. +const reflectValueReadOnlyMask = uint64(1<<5 | 1<<6) + // reflectValueRWAddr is equivalent to obj.Addr(), except that the returned // reflect.Value is usable in assignments even if obj was obtained by the use // of unexported struct fields. diff --git a/internal/state/encode.go b/internal/state/encode.go index 5ca5f3f..2edff56 100644 --- a/internal/state/encode.go +++ b/internal/state/encode.go @@ -769,7 +769,11 @@ func (es *encodeState) encodeObject(obj reflect.Value, how encodeStrategy, dest return } if !value.CanInterface() { - Failf("reflect.Value has restricted access") + // Clear access restrictions on this Value copy only. The restored + // view must retain them even though state can traverse private data. + flag := reflect.ValueOf(&value).Elem().FieldByName("flag") + encoded.ReadOnly = flag.Uint() & reflectValueReadOnlyMask + reflectValueRWAddr(flag).Elem().SetUint(flag.Uint() &^ reflectValueReadOnlyMask) } encoded.Addressable = value.CanAddr() if encoded.Addressable { diff --git a/internal/state/memory_test.go b/internal/state/memory_test.go index 32579c0..c3eb8d5 100644 --- a/internal/state/memory_test.go +++ b/internal/state/memory_test.go @@ -7,6 +7,7 @@ import ( "io" "math" "reflect" + "strings" "testing" ) @@ -50,6 +51,9 @@ func objectExamples() []object { &reflectTypeValue{Type: &reflectedType{ID: 1, reflectx: true}}, &reflectedValue{Type: typeSpecID(1), Value: intValue(42)}, &reflectedValue{Type: &pointerType{Type: typeSpecID(1)}, Value: &refValue{Root: 2}, Addressable: true}, + &reflectedValue{Type: typeSpecID(1), Value: intValue(42), ReadOnly: 1 << 5}, + &reflectedValue{Type: typeSpecID(1), Value: intValue(42), ReadOnly: 1 << 6}, + &reflectedValue{Type: &pointerType{Type: typeSpecID(1)}, Value: &refValue{Root: 2}, Addressable: true, ReadOnly: reflectValueReadOnlyMask}, &refValue{Root: 3, Dots: []dot{&f}, Type: closureType(0x520000)}, } } @@ -91,6 +95,19 @@ func TestMemoryObjects(t *testing.T) { } } +func TestMemoryReflectValueReadOnlyFlags(t *testing.T) { + for _, flags := range []uint64{1 << 7, 1 << 8, 1 << 63, ^uint64(0)} { + w := writer{mem: make([]byte, 128)} + if err := w.put(&reflectedValue{Type: typeSpecID(1), Value: intValue(42), ReadOnly: flags}); err != nil { + t.Fatal(err) + } + r := reader{mem: w.mem[:w.pos]} + if _, err := r.get(); err == nil || !strings.Contains(err.Error(), "invalid reflect.Value read-only flags") { + t.Fatalf("accepted flags %#x: %v", flags, err) + } + } +} + func TestMemoryBoundaries(t *testing.T) { for _, obj := range objectExamples() { w := writer{mem: make([]byte, 4096)} diff --git a/internal/state/object.go b/internal/state/object.go index b334741..db43f61 100644 --- a/internal/state/object.go +++ b/internal/state/object.go @@ -418,18 +418,24 @@ type reflectedValue struct { Type typeSpec Value object Addressable bool + ReadOnly uint64 } func (v *reflectedValue) save(w *writer) { saveTypeSpec(w, v.Type) boolValue(v.Addressable).save(w) + uintValue(v.ReadOnly).save(w) saveObject(w, v.Value) } func (*reflectedValue) load(r *reader) object { typ := loadTypeSpec(r) addressable := loadBool(r) - return &reflectedValue{Type: typ, Value: loadObject(r), Addressable: bool(addressable)} + readOnly := uint64(loadUint(r)) + if readOnly&^reflectValueReadOnlyMask != 0 { + Failf("invalid reflect.Value read-only flags %#x", readOnly) + } + return &reflectedValue{Type: typ, Value: loadObject(r), Addressable: bool(addressable), ReadOnly: readOnly} } // loadSlice loads an object of type sliceValue. diff --git a/internal/state/reflect_value_test.go b/internal/state/reflect_value_test.go index bd2cf39..3814bed 100644 --- a/internal/state/reflect_value_test.go +++ b/internal/state/reflect_value_test.go @@ -7,7 +7,6 @@ import ( "path/filepath" "reflect" "runtime" - "strings" "testing" ) @@ -174,18 +173,140 @@ func TestReflectValueLoadWait(t *testing.T) { } } -func TestReflectValueRestricted(t *testing.T) { - src := reflect.ValueOf(struct{ hidden int }{42}).Field(0) - if _, _, err := Save(context.Background(), make([]byte, 4096), &src); err == nil || !strings.Contains(err.Error(), "restricted access") { - t.Fatalf("restricted value: %v", err) +func TestReflectValuePrivateFields(t *testing.T) { + n := 42 + owner := struct { + number int + text string + pointer *int + slice []int + array [2]int + mapping map[string]int + iface any + nilIface any + }{42, "private", &n, []int{1, 2}, [2]int{3, 4}, map[string]int{"n": 5}, 6, nil} + for _, parent := range []reflect.Value{reflect.ValueOf(owner), reflect.ValueOf(&owner).Elem()} { + var src []reflect.Value + for i := 0; i < parent.NumField(); i++ { + src = append(src, parent.Field(i)) + } + var dst []reflect.Value + roundtrip(t, &src, &dst) + runtime.GC() + for i, value := range src { + got := dst[i] + if value.CanInterface() || got.CanInterface() || value.CanSet() || got.CanSet() || got.CanAddr() != value.CanAddr() || got.Type() != value.Type() { + t.Fatalf("field %d: private access or type changed", i) + } + for _, call := range []func(){func() { got.Interface() }, func() { got.Set(reflect.Zero(got.Type())) }} { + func() { + defer func() { + if recover() == nil { + t.Errorf("field %d: restricted operation succeeded", i) + } + }() + call() + }() + } + } + if dst[0].Int() != 42 || dst[1].String() != "private" || dst[2].Elem().Int() != 42 || dst[3].Index(1).Int() != 2 || dst[4].Index(1).Int() != 4 || dst[5].MapIndex(reflect.ValueOf("n")).Int() != 5 || dst[6].Elem().Int() != 6 || !dst[7].IsNil() { + t.Fatal("private field data changed") + } + if dst[2].Pointer() == reflect.ValueOf(&n).Pointer() { + t.Fatal("private pointer reused the source allocation") + } + } +} + +type reflectedPrivateEmbedded struct { + Exported int + hidden int +} + +func TestReflectValuePrivateEmbedded(t *testing.T) { + owner := struct { + reflectedPrivateEmbedded + named reflectedPrivateEmbedded + nested struct{ reflectedPrivateEmbedded } + }{ + reflectedPrivateEmbedded: reflectedPrivateEmbedded{1, 2}, + named: reflectedPrivateEmbedded{3, 4}, + nested: struct{ reflectedPrivateEmbedded }{reflectedPrivateEmbedded{5, 6}}, + } + for _, parent := range []reflect.Value{reflect.ValueOf(owner), reflect.ValueOf(&owner).Elem()} { + // EmbedRO alone permits access to Exported; StickyRO, including the + // combination from nested's private embedded field, must propagate. + src := []reflect.Value{parent.Field(0), parent.Field(1), parent.Field(2).Field(0)} + var dst []reflect.Value + roundtrip(t, &src, &dst) + for i, value := range src { + got := dst[i] + if value.CanInterface() || got.CanInterface() || got.CanSet() || got.CanAddr() != value.CanAddr() { + t.Fatalf("embedded value %d changed access", i) + } + for j := 0; j < value.NumField(); j++ { + before, after := value.Field(j), got.Field(j) + if before.CanInterface() != after.CanInterface() || before.CanSet() != after.CanSet() || before.Int() != after.Int() { + t.Fatalf("embedded value %d field %d lost access propagation", i, j) + } + if after.CanSet() { + after.SetInt(99) + if before.Int() == 99 { + t.Fatal("restored embedded field still aliases the source") + } + } + } + } + } +} + +func TestReflectValuePrivateRoundTrip(t *testing.T) { + type owner struct{ hidden int } + type root struct { + Owner *owner + Private reflect.Value + Writable reflect.Value + } + original := &owner{42} + host := root{original, reflect.ValueOf(original).Elem().Field(0), reflect.ValueOf(&original.hidden).Elem()} + var guest root + var hostState, guestState State + ctx := context.Background() + mem := make([]byte, 1<<20) + n, _, err := hostState.Save(ctx, mem, &host) + if err != nil { + t.Fatal(err) + } + if _, err := guestState.Load(ctx, mem[:n], &guest); err != nil { + t.Fatal(err) + } + if guest.Owner == original || guest.Private.UnsafeAddr() != guest.Writable.UnsafeAddr() { + t.Fatal("private field aliases were not relocated") + } + guest.Writable.SetInt(43) + if guest.Owner.hidden != 43 || guest.Private.Int() != 43 || original.hidden != 42 || guest.Private.CanInterface() || guest.Private.CanSet() { + t.Fatal("private view lost its value, restrictions or source isolation") + } + n, _, err = guestState.Save(ctx, mem, &guest) + if err != nil { + t.Fatal(err) + } + if _, err := hostState.Load(ctx, mem[:n], &host); err != nil { + t.Fatal(err) + } + runtime.GC() + if host.Owner != original || original.hidden != 43 || host.Private.Int() != 43 || host.Private.UnsafeAddr() != reflect.ValueOf(&original.hidden).Pointer() || host.Private.CanInterface() || host.Private.CanSet() || !host.Writable.CanSet() { + t.Fatal("private field writeback lost identity or access restrictions") } } func TestReflectValueNewProcess(t *testing.T) { type root struct { - Value reflect.Value - Field *int - Type reflect.Type + Value reflect.Value + Field *int + Type reflect.Type + Private reflect.Value + Hidden *int } const imageEnv = "SANDBOX_STATE_VALUE_TEST_IMAGE" if path := os.Getenv(imageEnv); path != "" { @@ -201,7 +322,11 @@ func TestReflectValueNewProcess(t *testing.T) { if dst.Value.Type() != dst.Type || dst.Value.Field(0).Addr().Interface().(*int) != dst.Field || *dst.Field != 42 { t.Fatal("reflected type, value or alias changed in the child") } + if dst.Private.Int() != 47 || dst.Private.CanInterface() || dst.Private.CanSet() || dst.Private.UnsafeAddr() != reflect.ValueOf(dst.Hidden).Pointer() { + t.Fatal("private field access or alias changed in the child") + } dst.Value.Field(0).SetInt(43) + *dst.Hidden = 48 output := make([]byte, 1<<20) n, _, err := Save(context.Background(), output, &dst) if err != nil { @@ -215,7 +340,11 @@ func TestReflectValueNewProcess(t *testing.T) { typ := reflect.StructOf([]reflect.StructField{{Name: "Count", Type: reflect.TypeFor[int](), Tag: `state:"value"`}}) value := reflect.New(typ).Elem() value.Field(0).SetInt(42) - src := root{Value: value, Field: value.Field(0).Addr().Interface().(*int), Type: typ} + hidden := struct{ hidden int }{47} + src := root{ + Value: value, Field: value.Field(0).Addr().Interface().(*int), Type: typ, + Private: reflect.ValueOf(&hidden).Elem().Field(0), Hidden: &hidden.hidden, + } mem := make([]byte, 1<<20) n, _, err := Save(context.Background(), mem, &src) if err != nil { @@ -241,4 +370,7 @@ func TestReflectValueNewProcess(t *testing.T) { if dst.Value.Field(0).Int() != 43 || dst.Value.Field(0).Addr().Interface().(*int) != dst.Field || *src.Field != 42 { t.Fatal("return transfer lost value, aliases or source isolation") } + if dst.Private.Int() != 48 || dst.Private.CanInterface() || dst.Private.CanSet() || dst.Private.UnsafeAddr() != reflect.ValueOf(dst.Hidden).Pointer() || hidden.hidden != 47 { + t.Fatal("return transfer lost private field data, aliases or access restrictions") + } }