Why this Golang code could leak value of variable which is in memory

Viewed 1125

This code could leak the value of the variable which is in memory.

I think maybe the fmt.XprintY doesn't reset the buffer, but my attempt at debugging is fruitless.

package main

import (
    "bytes"
    "fmt"
    "io"
    "text/template"
)

type SecWriter struct {
    w io.Writer
}

func (s *SecWriter) Write(p []byte) (n int, err error) {
    fmt.Println(string(p), len(p), cap(p))

    // here
    tmp := fmt.Sprintln("info{SSSSSSSSSSSSSSSSSSSSSSSSSSS}")
    if tmp == ""{}

    s.w.Write(p[:64])
    return 64, nil
}

func index() {
    exp := "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA{{1}}"

    b := &bytes.Buffer{}
    s := &SecWriter{
        w: b,
    }


    t := template.Must(template.New("index").Parse(exp))
    t.Execute(s, nil)

    fmt.Println("buf: ", b.String())
}

func main() {
    index()
}

My go env:

set GOARCH=amd64
set GOOS=windows

go version

go version go1.12.5 windows/amd64

And the output is:

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA 64 64
1 1 128
buf: AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1nfo{SSSSSSSSSSSSSSSSSSSSSSSSSSS}                 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAA

As you can see, part of the value of the variable in memory:

tmp := fmt.Sprintln("info{SSSSSSSSSSSSSSSSSSSSSSSSSSS}")

Leaks to the buffer.

3 Answers

In Go, the expression s.w.Write(p[:64]) can expand the slice beyond its length without an error (till the slice's capacity). In this case the provided buffer's length is only 1, but you expanded it to 64 (as in the second line of your output). The content in the extra 63 bytes is undefined, and it happens to be the output of some fmt methods.

The solution is to check the slice's length. If you want to make the slice foolproof and ensure it's impossible to see the content beyond its length, you can use the three index syntax for slices to set its capacity, such as p = p[::len(p)].

There is no memory leak, this proves it.
But there is an issue: The call to p := newPrinter() initialized here p.fmt.init(&p.buf) in func Fprint(w io.Writer, a ...interface{}) (n int, err error) returns the free memory (underlying array of a slice) without initializing it to zero (which is not initialized probably for performance reasons - which we expected to be all zero).

TL;DR:
Two solutions:
1. Workaround: Use s.w.Write(p) instead of s.w.Write(p[:64]), or edit your code and set p[len(p):cap(p)] all to zero (if you don't or cannot touch the 2nd solution):

func (s *SecWriter) Write(p []byte) (n int, err error) {
    b := p[len(p):cap(p)]
    for i := range b {
        b[i] = 0
    }
    fmt.Println(string(p), len(p), cap(p))
    // here
    tmp := fmt.Sprintln("info{SSSSSSSSSSSSSSSSSSSSSSSSSSS}")
    if tmp == "" {
    }
    s.w.Write(p[:64])
    return 64, nil
}

  1. In Windows (C:\Go\src\fmt\format.go) or Linux (/usr/local/go/src/fmt/format.go) file at line 58 set the buffer to all zero:
    b := (*buf)[:cap(*buf)]
    for i := range b {
        b[i] = 0
    }

Inside this function:

func (f *fmt) init(buf *buffer) {
    b := (*buf)[:cap(*buf)]
    for i := range b {
        b[i] = 0
    }
    f.buf = buf
    f.clearflags()
}

Your code output with this applied:

AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA 64 64
1 1 128
buf:  AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA1

Long answer:
You're watching the slice data beyond the assigned length, and you are allowed to see the slice data to the slice capacity.
You may replace: m.Writer.Write(p[:8]) with: m.Writer.Write(p) which makes your code works correctly. The following code shows that template.Parse() tokenizes the template to 3 parts and calls my.Write() three times. the interesting part here is the second call to the my.Write() shows a compiler-generated slice with different slice capacity which is not initialized to zero, "maybe this is a minor harmless issue":

If you want to spy on your computer's memory, try this:

package main

import (
    "bytes"
    "fmt"
    "io"
    "text/template"
)

func main() {
    buf := &bytes.Buffer{}
    my := &myWriter{"You", buf}
    template.Must(template.New("my").Parse("Hi{{.Name}}Bye.")).Execute(my, my)
    fmt.Printf("<<%q>>\n", buf.String())
}
func (m *myWriter) Write(p []byte) (n int, err error) {
    fmt.Printf("len=%v cap=%v\t%v %v\n", len(p), cap(p), string(p), p[:cap(p)])
    no++
    fmt.Println("gen:", no, gen())
    m.Writer.Write(p)
    // m.Writer.Write(p[:8])
    return 8, nil
}

type myWriter struct {
    Name string
    io.Writer
}

const genLen = 8

func gen() string {
    b := [genLen]byte{}
    for i := range b {
        b[i] = no
    }
    return string(b[:])
}

var no = byte(49) //'1'

Output:

len=2 cap=8 Hi [72 105 0 0 0 0 0 0]
gen: 50 22222222
len=3 cap=64    You [89 111 117 58 32 53 48 32 50 50 50 50 50 50 50 50 10 50 32 49 48 53 32 48 32 48 32 48 32 48 32 48 32 48 93 10 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]
gen: 51 33333333
len=4 cap=8 Bye. [66 121 101 46 0 0 0 0]
gen: 52 44444444
<<"HiYouBye.">>

And change const genLen = 64 try this intersting: the cap=64 changes to cap=128 (which is not expected):

Output:

len=2 cap=8 Hi [72 105 0 0 0 0 0 0]
gen: 50 2222222222222222222222222222222222222222222222222222222222222222
len=3 cap=128   You [89 111 117 58 32 53 48 32 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 50 10 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]
gen: 51 3333333333333333333333333333333333333333333333333333333333333333
len=4 cap=8 Bye. [66 121 101 46 0 0 0 0]
gen: 52 4444444444444444444444444444444444444444444444444444444444444444
<<"HiYouBye.">>

The t.Execute(my, my) calls func (m *myWriter) Write(p []byte) so the p with len=3 and cap=128 generated by the tamplate engine.

After debugging the 2nd code inside /usr/local/go/src/fmt/print.go file at line 230, it seems it is fmt.buffer with length=3, and cap=128, here:

func Fprint(w io.Writer, a ...interface{}) (n int, err error) {
    p := newPrinter()
    p.doPrint(a)
    n, err = w.Write(p.buf)
    p.free()
    return
}

The call to p := newPrinter() initialized here p.fmt.init(&p.buf):

// newPrinter allocates a new pp struct or grabs a cached one.
func newPrinter() *pp {
    p := ppFree.Get().(*pp)
    p.panicking = false
    p.erroring = false
    p.wrapErrs = false
    p.fmt.init(&p.buf)
    return p
}

Gets and returns the free memory without initializing it to zero.

If you assign to the variable directly instead of doing fmt.Sprintln, the variable will not leak.

Code: https://play.golang.org/p/Nz0y_MfDjP1

So I believe fmt.Sprintln is causing the leak. This function calls a another unexported function newPrinter to get a printer which in turn maintains it's own pool and caches. I haven't gone deep enough but my guess would be somehow the buffer you created manually might be getting overlapped / shared here somehow.

(I will update the answer if I find anything else)

Related