Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
stevemk14ebr opened this issue Aug 20, 2021 · 1 comment
Closed

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

stevemk14ebr opened this issue Aug 20, 2021 · 1 comment
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@stevemk14ebr
Copy link
Contributor

stevemk14ebr commented Aug 20, 2021 •
edited

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.
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 20, 2021
@mknyszek mknyszek added this to the Backlog milestone Aug 20, 2021
@mknyszek mknyszek changed the title OOM in buildid computation when buildid is stripped cmd/buildid: OOM in buildid computation when buildid is stripped Aug 20, 2021
@gopherbot
Copy link

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

@rsc rsc changed the title cmd/buildid: OOM in buildid computation when buildid is stripped cmd/internal/buildid: FindAndHash loops forever with empty id Aug 20, 2021
@golang golang locked and limited conversation to collaborators Aug 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@mknyszek @stevemk14ebr @gopherbot