cmd/internal/buildid: FindAndHash loops forever with empty id

[stevemk14ebr]

Hello the code in question is here:

go/src/cmd/buildid/buildid.go

Lines 35 to 50 in 2ebe77a

	id, err := buildid.ReadFile(file)
	if err != nil {
	log.Fatal(err)
	}
	if !*wflag {
	fmt.Printf("%s\n", id)
	return
	}
	// Keep in sync with src/cmd/go/internal/work/buildid.go:updateBuildID
	f, err := os.Open(file)
	if err != nil {
	log.Fatal(err)
	}
	matches, hash, err := buildid.FindAndHash(f, id, 0)

If the buildid of a binary is modified manually to be empty, ie "" then the FindAndHash routine will exhaust system memory and DOS the compiler. The issue is that the matches array built up here:

go/src/cmd/internal/buildid/rewrite.go

Lines 57 to 66 in 2ebe77a

	for {
	i := bytes.Index(buf[start:tiny+n], idBytes)
	if i < 0 {
	break
	}
	matches = append(matches, offset+int64(start+i-tiny))
	h.Write(buf[start : start+i])
	h.Write(zeros)
	start += i + len(id)
	}

will search with IdBytes equal to an empty array. This empty array will match every single time returning and idx of 0 and so every loop iteration will append into the temporary. To fix this a guard simple need to be added before calling FindAndHash to verify that the id is != ""

When is this actually an issue? Probably never, but malicious attackers do modify binaries after compilation and if anyone re-uses this code (such as I do) then they will be exposed to this bug. So essentially most common cases will not exercise this logic flaw but it does exist, so a guard should be added regardless imho.

[gopherbot]

Change https://golang.org/cl/344049 mentions this issue: cmd/internal/buildid: reject empty id