cmd/pack: fix export data truncation bug

The binary export data format includes escaping to prevent "\n$$" from
appearing internally, but not "\n!\n". This could result in a false
positive when cmd/pack searched for "\n!\n" as the delimiter between
package definition and linker object.

To address this, this CL changes cmd/pack to also be aware of the
"\n$$" markers, and to ignore "\n!\n" within the export data.

Fixes #21703.

Change-Id: I71ea8ba49dbd066c7afb7717ddc0190e38fe5649
Reviewed-on: https://go-review.googlesource.com/60773
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

2 files changed, 42 insertions, 1 deletions

diff --git a/src/cmd/pack/pack.go b/src/cmd/pack/pack.go
index 1c168f946b..3abc83e090 100644
--- a/src/cmd/pack/pack.go
+++ b/src/cmd/pack/pack.go
@@ -426,8 +426,15 @@ func readPkgdef(file string) (data []byte, err error) {
 	// Read from file, collecting header for __.PKGDEF.
 	// The header is from the beginning of the file until a line
 	// containing just "!". The first line must begin with "go object ".
+	//
+	// Note: It's possible for "\n!\n" to appear within the binary
+	// package export data format. To avoid truncating the package
+	// definition prematurely (issue 21703), we keep keep track of
+	// how many "$$" delimiters we've seen.
+
 	rbuf := bufio.NewReader(f)
 	var wbuf bytes.Buffer
+	markers := 0
 	for {
 		line, err := rbuf.ReadBytes('\n')
 		if err != nil {
@@ -436,9 +443,12 @@ func readPkgdef(file string) (data []byte, err error) {
 		if wbuf.Len() == 0 && !bytes.HasPrefix(line, []byte("go object ")) {
 			return nil, errors.New("not a Go object file")
 		}
-		if bytes.Equal(line, []byte("!\n")) {
+		if markers%2 == 0 && bytes.Equal(line, []byte("!\n")) {
 			break
 		}
+		if bytes.HasPrefix(line, []byte("$$")) {
+			markers++
+		}
 		wbuf.Write(line)
 	}
 	return wbuf.Bytes(), nil
diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go
index 79d9cde292..b2217c090f 100644
--- a/src/cmd/pack/pack_test.go
+++ b/src/cmd/pack/pack_test.go
@@ -295,6 +295,37 @@ func TestLargeDefs(t *testing.T) {
 	}
 }
 
+// Test that "\n!\n" inside export data doesn't result in a truncated
+// package definition when creating a .a archive from a .o Go object.
+func TestIssue21703(t *testing.T) {
+	testenv.MustHaveGoBuild(t)
+
+	dir := tmpDir(t)
+	defer os.RemoveAll(dir)
+
+	const aSrc = `package a; const X = "\n!\n"`
+	err := ioutil.WriteFile(filepath.Join(dir, "a.go"), []byte(aSrc), 0666)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	const bSrc = `package b; import _ "a"`
+	err = ioutil.WriteFile(filepath.Join(dir, "b.go"), []byte(bSrc), 0666)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	run := func(args ...string) string {
+		return doRun(t, dir, args...)
+	}
+
+	goBin := testenv.GoToolPath(t)
+	run(goBin, "build", "cmd/pack") // writes pack binary to dir
+	run(goBin, "tool", "compile", "a.go")
+	run("./pack", "c", "a.a", "a.o")
+	run(goBin, "tool", "compile", "-I", ".", "b.go")
+}
+
 // doRun runs a program in a directory and returns the output.
 func doRun(t *testing.T, dir string, args ...string) string {
 	cmd := exec.Command(args[0], args[1:]...)