cmd/compile: give every really deep type a unique name

This avoids the security problem in #29312 where two very deep, but
distinct, types are given the same name. They both make it to the
linker which chooses one, and the use of the other is now type unsafe.

Instead, give every very deep type its own name. This errs on the
other side, in that very deep types that should be convertible to each
other might now not be. But at least that's not a security hole.

Update #29312.

Change-Id: Iac0ebe73fdc50594fd6fbf7432eef65f9a053126
Reviewed-on: https://go-review.googlesource.com/c/go/+/213517
Run-TryBot: Keith Randall <khr@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>

diff --git a/src/cmd/compile/internal/gc/fmt.go b/src/cmd/compile/internal/gc/fmt.go
index 95576b63955090965f7aff836daafab157c51e7f..d4104fdd6206225e42dff5d794a13381871e3ac2 100644 (file)
--- a/src/cmd/compile/internal/gc/fmt.go
+++ b/src/cmd/compile/internal/gc/fmt.go
@@ -1731,6 +1731,8 @@ func typeFormat(t *types.Type, s fmt.State, verb rune, mode fmtMode) {
        }
 }
 
+var deepTypes map[*types.Type]string
+
 // See #16897 before changing the implementation of tconv.
 func tconv(t *types.Type, flag FmtFlag, mode fmtMode, depth int) string {
        if t == nil {
@@ -1747,8 +1749,19 @@ func tconv(t *types.Type, flag FmtFlag, mode fmtMode, depth int) string {
        // limits the depths of valid composite types, but they are likely
        // artificially created.
        // TODO(gri) should have proper cycle detection here, eventually (issue #29312)
+       // For now, ensure that each of these really deep types are at least uniquely
+       // named, so that such types don't collide in the linker and thus allow security holes.
        if depth > 250 {
-               return "<...>"
+               if str := deepTypes[t]; str != "" {
+                       return str
+               }
+               if deepTypes == nil {
+                       deepTypes = map[*types.Type]string{}
+               }
+               id := len(deepTypes)
+               str := fmt.Sprintf("<...uniquetype_%d_in_%s>", id, curpkg().Path)
+               deepTypes[t] = str
+               return str
        }
 
        flag, mode = flag.update(mode)

diff --git a/test/fixedbugs/issue29312.go b/test/fixedbugs/issue29312.go
new file mode 100644 (file)
index 0000000..4293e01
--- /dev/null
+++ b/test/fixedbugs/issue29312.go
@@ -0,0 +1,70 @@
+// run
+
+// Copyright 2020 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This test is not for a fix of 29312 proper, but for the patch that
+// makes sure we at least don't have a security hole because of 29312.
+
+// This code generates lots of types. The binary should contain
+// a runtime.slicetype for each of the following 253 types:
+//
+//     []*pwn
+//     [][]*pwn
+//     ...
+//     [][]...[][]*pwn          - 249 total "[]"
+//     [][]...[][][]*pwn        - 250 total "[]"
+//     [][]...[][][][]*pwn      - 251 total "[]"
+//     [][]...[][][][][]*pwn    - 252 total "[]"
+//     [][]...[][][][][][]*pwn  - 253 total "[]"
+//
+// The type names for these types are as follows. Because we truncate
+// the name at depth 250, the last few names are all identical:
+//
+//     type.[]*"".pwn
+//     type.[][]*"".pwn
+//     ...
+//     type.[][]...[][]*pwn       - 249 total "[]"
+//     type.[][]...[][][]*<...>   - 250 total "[]"
+//     type.[][]...[][][][]<...>  - 251 total "[]"
+//     type.[][]...[][][][]<...>  - 252 total "[]" (but only 251 "[]" in the name)
+//     type.[][]...[][][][]<...>  - 253 total "[]" (but only 251 "[]" in the name)
+//
+// Because the names of the last 3 types are all identical, the
+// compiler will generate only a single runtime.slicetype data
+// structure for all 3 underlying types. It turns out the compiler
+// generates just the 251-entry one. There aren't any
+// runtime.slicetypes generated for the final two types.
+//
+// The compiler passes type.[]...[]<...> (251 total "[]") to
+// fmt.Sprintf (instead of the correct 253 one). But the data
+// structure at runtime actually has 253 nesting levels. So we end up
+// calling String on something that is of type [][]*pwn instead of
+// something of type *pwn. The way arg passing in Go works, the
+// backing store pointer for the outer slice becomes the "this"
+// pointer of the String method, which points to the inner []*pwn
+// slice.  The String method then modifies the length of that inner
+// slice.
+package main
+
+import "fmt"
+
+type pwn struct {
+       a [3]uint
+}
+
+func (this *pwn) String() string {
+       this.a[1] = 7 // update length
+       return ""
+}
+
+func main() {
+       var a pwn
+       s := [][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][][]*pwn{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{&a}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}} // depth 253
+       fmt.Sprint(s)
+       n := len(s[0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0][0]) // depth 252, type []*pwn
+       if n != 1 {
+               panic(fmt.Sprintf("length was changed, want 1 got %d", n))
+       }
+}