]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: properly frame panic values in tracebacks
authorAlan Donovan <adonovan@google.com>
Tue, 23 Apr 2024 16:44:54 +0000 (12:44 -0400)
committerGopher Robot <gobot@golang.org>
Wed, 8 May 2024 19:10:41 +0000 (19:10 +0000)
This CL causes the printing of panic values to ensure that all
newlines in the output are immediately followed by a tab, so
that there is no way for a maliciously crafted panic value to
fool a program attempting to parse the traceback into thinking
that the panic value is in fact a goroutine stack.

See https://github.com/golang/go/issues/64590#issuecomment-1932675696

+ release note

Updates #64590
Updates #63455

Change-Id: I5142acb777383c0c122779d984e73879567dc627
Reviewed-on: https://go-review.googlesource.com/c/go/+/581215
Auto-Submit: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
doc/next/4-runtime.md
src/runtime/crash_test.go
src/runtime/error.go
src/runtime/panic.go
src/runtime/panic_test.go
src/runtime/testdata/testprog/crash.go
src/runtime/testdata/testprog/panicprint.go
src/testing/testing_test.go

index 1f8e445e0b10dec62dbb8c5f06d35ece0db1cfdd..7553154a162b0792c3dc91b917bd51ade14e1948 100644 (file)
@@ -1 +1,7 @@
 ## Runtime {#runtime}
+
+The traceback printed by the runtime after an unhandled panic or other
+fatal error now indents the second and subsequent lines of the error
+message (for example, the argument to panic) by a single tab, so that
+it can be unambiguously distinguished from the stack trace of the
+first goroutine. See [#64590](/issue/64590) for discussion.
index 19c9cddf36f10f090d34ae1e37bf379264b96e46..9a5fa6158815af94ba2e00d15a0ba2b731c43528 100644 (file)
@@ -966,11 +966,11 @@ func TestPanicWhilePanicking(t *testing.T) {
                Func string
        }{
                {
-                       "panic while printing panic value: important error message",
+                       "panic while printing panic value: important multi-line\n\terror message",
                        "ErrorPanic",
                },
                {
-                       "panic while printing panic value: important stringer message",
+                       "panic while printing panic value: important multi-line\n\tstringer message",
                        "StringerPanic",
                },
                {
@@ -986,7 +986,7 @@ func TestPanicWhilePanicking(t *testing.T) {
                        "CircularPanic",
                },
                {
-                       "important string message",
+                       "important multi-line\n\tstring message",
                        "StringPanic",
                },
                {
index fe95f310058a0fcd3b881d4f0493298739de0385..406f36ca5f9d37313bf3c010b165e9a135290f4b 100644 (file)
@@ -211,11 +211,16 @@ type stringer interface {
        String() string
 }
 
-// printany prints an argument passed to panic.
+// printpanicval prints an argument passed to panic.
 // If panic is called with a value that has a String or Error method,
 // it has already been converted into a string by preprintpanics.
-func printany(i any) {
-       switch v := i.(type) {
+//
+// To ensure that the traceback can be unambiguously parsed even when
+// the panic value contains "\ngoroutine" and other stack-like
+// strings, newlines in the string representation of v are replaced by
+// "\n\t".
+func printpanicval(v any) {
+       switch v := v.(type) {
        case nil:
                print("nil")
        case bool:
@@ -251,19 +256,22 @@ func printany(i any) {
        case complex128:
                print(v)
        case string:
-               print(v)
+               printindented(v)
        default:
-               printanycustomtype(i)
+               printanycustomtype(v)
        }
 }
 
+// Invariant: each newline in the string representation is followed by a tab.
 func printanycustomtype(i any) {
        eface := efaceOf(&i)
        typestring := toRType(eface._type).string()
 
        switch eface._type.Kind_ {
        case abi.String:
-               print(typestring, `("`, *(*string)(eface.data), `")`)
+               print(typestring, `("`)
+               printindented(*(*string)(eface.data))
+               print(`")`)
        case abi.Bool:
                print(typestring, "(", *(*bool)(eface.data), ")")
        case abi.Int:
@@ -301,6 +309,21 @@ func printanycustomtype(i any) {
        }
 }
 
+// printindented prints s, replacing "\n" with "\n\t".
+func printindented(s string) {
+       for {
+               i := bytealg.IndexByteString(s, '\n')
+               if i < 0 {
+                       break
+               }
+               i += len("\n")
+               print(s[:i])
+               print("\t")
+               s = s[i:]
+       }
+       print(s)
+}
+
 // panicwrap generates a panic for a call to a wrapped value method
 // with a nil pointer receiver.
 //
index 51b57520c1875e15c39f9120623179deaa7587a6..27fcf73ff49883e003ab4962d50efa6795aa6acd 100644 (file)
@@ -656,7 +656,7 @@ func printpanics(p *_panic) {
                return
        }
        print("panic: ")
-       printany(p.arg)
+       printpanicval(p.arg)
        if p.recovered {
                print(" [recovered]")
        }
@@ -718,20 +718,20 @@ func gopanic(e any) {
        gp := getg()
        if gp.m.curg != gp {
                print("panic: ")
-               printany(e)
+               printpanicval(e)
                print("\n")
                throw("panic on system stack")
        }
 
        if gp.m.mallocing != 0 {
                print("panic: ")
-               printany(e)
+               printpanicval(e)
                print("\n")
                throw("panic during malloc")
        }
        if gp.m.preemptoff != "" {
                print("panic: ")
-               printany(e)
+               printpanicval(e)
                print("\n")
                print("preempt off reason: ")
                print(gp.m.preemptoff)
@@ -740,7 +740,7 @@ func gopanic(e any) {
        }
        if gp.m.locks != 0 {
                print("panic: ")
-               printany(e)
+               printpanicval(e)
                print("\n")
                throw("panic holding locks")
        }
@@ -1015,7 +1015,9 @@ func throw(s string) {
        // Everything throw does should be recursively nosplit so it
        // can be called even when it's unsafe to grow the stack.
        systemstack(func() {
-               print("fatal error: ", s, "\n")
+               print("fatal error: ")
+               printpanicval(s)
+               print("\n")
        })
 
        fatalthrow(throwTypeRuntime)
@@ -1034,7 +1036,9 @@ func fatal(s string) {
        // Everything fatal does should be recursively nosplit so it
        // can be called even when it's unsafe to grow the stack.
        systemstack(func() {
-               print("fatal error: ", s, "\n")
+               print("fatal error: ")
+               printpanicval(s)
+               print("\n")
        })
 
        fatalthrow(throwTypeUser)
index b8a300f6b10c20366eac2002911b4b27eefc0c7c..994abfdd4559372627bfa6f70ddc5e56b05510fa 100644 (file)
@@ -27,7 +27,7 @@ func TestPanicWithDirectlyPrintableCustomTypes(t *testing.T) {
                {"panicCustomInt16", `panic: main.MyInt16(93)`},
                {"panicCustomInt32", `panic: main.MyInt32(93)`},
                {"panicCustomInt64", `panic: main.MyInt64(93)`},
-               {"panicCustomString", `panic: main.MyString("Panic")`},
+               {"panicCustomString", `panic: main.MyString("Panic` + "\n\t" + `line two")`},
                {"panicCustomUint", `panic: main.MyUint(93)`},
                {"panicCustomUint8", `panic: main.MyUint8(93)`},
                {"panicCustomUint16", `panic: main.MyUint16(93)`},
index 38c8f6a2fa26b3b8481b3831a2f4596342e62514..bdc395f652edcac538e04c502f6ff7788d87019a 100644 (file)
@@ -77,7 +77,7 @@ func DoublePanic() {
 type exampleError struct{}
 
 func (e exampleError) Error() string {
-       panic("important error message")
+       panic("important multi-line\nerror message")
 }
 
 func ErrorPanic() {
@@ -97,7 +97,7 @@ func DoubleErrorPanic() {
 type exampleStringer struct{}
 
 func (s exampleStringer) String() string {
-       panic("important stringer message")
+       panic("important multi-line\nstringer message")
 }
 
 func StringerPanic() {
@@ -115,7 +115,7 @@ func DoubleStringerPanic() {
 }
 
 func StringPanic() {
-       panic("important string message")
+       panic("important multi-line\nstring message")
 }
 
 func NilPanic() {
index c8deabe2ab186fe788034f93e0e0c06a50d88d60..4ce958ba3dbacc9f1b972f80465d9eb38ecf6cd0 100644 (file)
@@ -31,7 +31,7 @@ func panicCustomComplex128() {
 }
 
 func panicCustomString() {
-       panic(MyString("Panic"))
+       panic(MyString("Panic\nline two"))
 }
 
 func panicCustomBool() {
index d3822dfd578f803bf45cb196429f0ff997ce6016..4a9303952e79caadee427e6e36d589afb74db885 100644 (file)
@@ -762,7 +762,8 @@ func parseRunningTests(out []byte) (runningTests []string, ok bool) {
        inRunningTests := false
        for _, line := range strings.Split(string(out), "\n") {
                if inRunningTests {
-                       if trimmed, ok := strings.CutPrefix(line, "\t"); ok {
+                       // Package testing adds one tab, the panic printer adds another.
+                       if trimmed, ok := strings.CutPrefix(line, "\t\t"); ok {
                                if name, _, ok := strings.Cut(trimmed, " "); ok {
                                        runningTests = append(runningTests, name)
                                        continue