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 <[email protected]> Run-TryBot: Jonathan Amsterdam <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Ian Cottrell <[email protected]>
apstndb · Mar 29, 2021 · 794f42d · 794f42d
1 parent 48f44c5
commit 794f42d
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 3 deletions.
diff --git 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
@@ -7,7 +7,9 @@ package label_test
 import (
 	"bytes"
 	"fmt"
+	"runtime"
 	"testing"
+	"unsafe"
 
 	"github.com/apstndb/gotoolsdiff/internal/event/keys"
 	"github.com/apstndb/gotoolsdiff/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()
+}