Go: cmd/compile: passing method on value receiver causes memory corruption

[dsnet]

Using go1.7.4, but seems to have been present since go1.3
OS: Linux carbonite 3.13.0-37-generic #64-Ubuntu SMP Mon Sep 22 21:28:38 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

Consider the following code:

type X struct {
    A, B []byte
}

func (x X) Print() {
    fmt.Printf("%p %p\n", x.A, x.B)
}

func caller(f func()) {
    f()
}

func main() {
    caller(X{A: []byte{}}.Print)
}

Currently this prints:

0x516cf0 0xc420064000

I expect this to print:

0x516cf0 0x0

The Print method is declared on a value receiver. In the main function we define a struct literal of type X and explicitly set the A field to be some value, but we expect the B field to be implicitly be zeroed out. The Print method of that struct literal is passed to another function as a callback.

When the callback is called, it shows that the B field is not zeroed and contains a pointer to some arbitrary piece of memory.

\cc @randall77 @dr2chase @paranoiacblack

[dsnet]

Marking as Go1.8maybe for now since this seems significant (discovered in production code). Feel free to mark as Go1.9.

[griesemer]

I don't think we can fix this at this stage of Go1.8. Also, it's not a regression (looks like the bug was there in 1.4). Finally, there's a work-around (provide an explicit closure):

package main

import (
    "fmt"
)

type X struct {
    A, B *int
}

func (t X) Print() {
    fmt.Printf("%p %p\n", t.A, t.B)
}

func caller(f func()) {
    f()
}

func main() {
    caller(X{A: nil}.Print)
    caller(func() { X{A: nil}.Print() })
}

[odeke-em]

Hold up a second, on the playground running go1.7.4, the program prints out the right output that @dsnet expects ie https://play.golang.org/p/X6YL_-v-TB therefore I wonder what's really different, perhaps an OS thing?

but in deed it fails on tip on OSX

$ go run X6YL_-v-TB.go 
0x1128a80 0xc420018360
runtime.Version: devel +0937441 Tue Dec 20 02:44:01 2016 -0800
$ uname -a
Darwin Emmanuels-MBP-2 15.6.0 Darwin Kernel Version 15.6.0: Mon Aug 29 20:21:34 PDT 2016; root:xnu-3248.60.11~1/RELEASE_X86_64 x86_64

[dgryski]

@odeke-em That might have more to do with stack layout and the NaCl calling convention.

[minux]

This seems like a pretty bad security issue.
the B slice is picking up garbage on the stack frame, so
it might actually gives the method a way to read/write
arbitrary memory locations, and it might also cause
GC/stack copy to crash on invalid pointers.

For example, I added a Printf of len(t.B) and cap(t.B),
and both values on my systems are: 0xc4200001a0.

[dgryski]

@minux When I was playing with this earlier, it's looked like stack allocated objects aren't cleared, but heap allocated ones were.

[dgryski]

This can be used to overwrite the stack and control the return address:

Code here: https://play.golang.org/p/V37IeKl5pN

<dgryski@kamek[18410] \ʕ◔ϖ◔ʔ/ > gdb ./18410 
GNU gdb (Ubuntu 7.11.90.20161005-0ubuntu1) 7.11.90.20161005-git
Copyright (C) 2016 Free Software Foundation, Inc.
...
Reading symbols from ./18410...done.
(gdb) r
Starting program: /home/dgryski/Dropbox/GITS/gocode/src/github.com/dgryski/18410/18410 
[New LWP 7931]
[New LWP 7932]
[New LWP 7933]
[New LWP 7934]
842350461344 842350461344

Thread 1 "18410" received signal SIGSEGV, Segmentation fault.
runtime.memmove () at /home/dgryski/work/src/cvs/go/src/runtime/memmove_amd64.s:147
147     RET
(gdb) bt 10
#0  runtime.memmove () at /home/dgryski/work/src/cvs/go/src/runtime/memmove_amd64.s:147
#1  0x4141414141414141 in ?? ()
#2  0x4141414141414141 in ?? ()
#3  0x4141414141414141 in ?? ()
#4  0x4141414141414141 in ?? ()
#5  0x4141414141414141 in ?? ()
#6  0x4141414141414141 in ?? ()
#7  0x4141414141414141 in ?? ()
#8  0x4141414141414141 in ?? ()
#9  0x4141414141414141 in ?? ()
(More stack frames follow...)

Playground output:

272801680 272630000
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0xffffffff addr=0x0 pc=0x8c3ac]

goroutine 1 [running]:
panic(0x9b700, 0x1040a038)
    /usr/local/go/src/runtime/panic.go:500 +0x720
main.(X).(main.setB)-fm(0x10440000, 0x40000, 0x40000, 0x0)
    /tmp/sandbox133984929/main.go:21 +0x1a0
main.caller(0x10429f6c, 0x104000f0)
    /tmp/sandbox133984929/main.go:17 +0xc0
main.main()
    /tmp/sandbox133984929/main.go:21 +0xa0

Program exited.

[minux]

It will be a fun exercise to use this vulnerability to do a ROP demo
on the playground.

Now I hope this hole is not closed too fast as I haven't had time
to play with this idea yet.....

(Note: I don't thinking such an "attack" will affect the real security
of the playground.)

[dgryski]

No, but it probably affects appengine which is still on 1.6.3.

[dr2chase]

Problem has (something) to do with treatment of STRUCTLIT under a CALLPART, vs under AS to a target.

.   CALLFUNC-list
.   .   CALLPART l(25) esc(no) tc(1) main.X.Print FUNC-func()
.   .   .   STRUCTLIT l(25) tc(1) main.X
.   .   .   .   TYPE main.X u(1) a(true) l(7) x(0) class(PEXTERN) tc(1) type=main.X main.X main.X
.   .   .   STRUCTLIT-list
.   .   .   .   STRUCTKEY l(25) x(0) main.A
.   .   .   .   .   ADDR l(25) esc(h) tc(1) PTR64-*int
.   .   .   .   .   .   NAME-main.i u(2) a(true) g(1) l(22) x(0) class(PAUTOHEAP) f(1) esc(h) tc(1) addrtaken used(true) int

.   .   .   .   STRUCTKEY l(25) x(16) main.C
.   .   .   .   .   ADDR l(25) esc(h) tc(1) PTR64-*int
.   .   .   .   .   .   NAME-main.j u(2) a(true) g(2) l(22) x(0) class(PAUTOHEAP) f(1) esc(h) tc(1) addrtaken used(true) int
.   .   .   NAME-main.Print u(1) a(true) l(25) x(0)

[gopherbot]

CL https://golang.org/cl/34622 mentions this issue.

[gopherbot]

CL https://golang.org/cl/35636 mentions this issue.