From 2c039f7ffc21afa767104895e0bad5b04ea4d340 Mon Sep 17 00:00:00 2001 From: Jonathan Amsterdam Date: Sun, 28 Mar 2021 18:58:35 -0400 Subject: [PATCH] internal/event/label: prevent unsafe get of non-string Declare an unexported type and use it in OfString/UnpackString so it is impossible to fool the Label.UnpackString into accessing a non-string. Change-Id: I840fcc99590e532a78a5f9a416cd40ce9ec2163a Reviewed-on: https://go-review.googlesource.com/c/tools/+/305309 Trust: Jonathan Amsterdam Run-TryBot: Jonathan Amsterdam gopls-CI: kokoro TryBot-Result: Go Bot Reviewed-by: Ian Cottrell --- internal/event/label/label.go | 8 +++++--- internal/event/label/label_test.go | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/internal/event/label/label.go b/internal/event/label/label.go index b55c12eb25..0f526e1f9a 100644 --- a/internal/event/label/label.go +++ b/internal/event/label/label.go @@ -96,6 +96,8 @@ func Of64(k Key, v uint64) Label { return Label{key: k, packed: v} } // access should be done with the From method of the key. func (t Label) Unpack64() uint64 { return t.packed } +type stringptr unsafe.Pointer + // OfString creates a new label from a key and a string. // This method is for implementing new key types, label creation should // normally be done with the Of method of the key. @@ -104,7 +106,7 @@ func OfString(k Key, v string) Label { return Label{ key: k, packed: uint64(hdr.Len), - untyped: unsafe.Pointer(hdr.Data), + untyped: stringptr(hdr.Data), } } @@ -115,9 +117,9 @@ func OfString(k Key, v string) Label { func (t Label) UnpackString() string { var v string hdr := (*reflect.StringHeader)(unsafe.Pointer(&v)) - hdr.Data = uintptr(t.untyped.(unsafe.Pointer)) + hdr.Data = uintptr(t.untyped.(stringptr)) hdr.Len = int(t.packed) - return *(*string)(unsafe.Pointer(hdr)) + return v } // Valid returns true if the Label is a valid one (it has a key). diff --git a/internal/event/label/label_test.go b/internal/event/label/label_test.go index ea34ff7d31..a2b58185b7 100644 --- a/internal/event/label/label_test.go +++ b/internal/event/label/label_test.go @@ -7,7 +7,9 @@ package label_test import ( "bytes" "fmt" + "runtime" "testing" + "unsafe" "golang.org/x/tools/internal/event/keys" "golang.org/x/tools/internal/event/label" @@ -267,3 +269,17 @@ func printMap(lm label.Map, keys []label.Key) string { } return buf.String() } + +func TestAttemptedStringCorruption(t *testing.T) { + defer func() { + r := recover() + if _, ok := r.(*runtime.TypeAssertionError); !ok { + t.Fatalf("wanted to recover TypeAssertionError, got %T", r) + } + }() + + var x uint64 = 12390 + p := unsafe.Pointer(&x) + l := label.OfValue(AKey, p) + _ = l.UnpackString() +}