Skip to content

cmd/internal/buildid: buildid invalid read when program alignment is zero #62097

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
Closed
stevemk14ebr opened this issue Aug 17, 2023 · 6 comments
Closed

cmd/internal/buildid: buildid invalid read when program alignment is zero #62097

stevemk14ebr opened this issue Aug 17, 2023 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.22

Comments

@stevemk14ebr
Copy link
Contributor

What version of Go are you using (go version)?

$ go version
1.20.2

Does this issue reproduce with the latest release?

Yes

What did you do?

Using binary 2a2513e8687993ed71cc48b09676abf18bb0e739f15473d36c50627b05036686 (on VT) running the buildid code over the sample fails to correctly read the notes section. This is not a Go binary, however this does excersize a failure case in the build id code here:

//note.go readELF

off += notesz
align := p.Align
alignedOff := (off + align - 1) &^ (align - 1)
notesz += alignedOff - off
off = alignedOff
filesz -= notesz
note = note[notesz:]

image

The alignedOff is aligned to zero while the offset is positive leading to an underflow when the notesz is calculated. This later leads to an invalid array access at note = note[notesz:]

To fix, the code should ignore alignment when it's zero:

off += notesz
align := p.Align
if align != 0 {
    alignedOff := (off + align - 1) &^ (align - 1)
    notesz += alignedOff - off
    off = alignedOff
}
filesz -= notesz
note = note[notesz:]
@dmitshur
Copy link
Contributor

dmitshur commented Aug 17, 2023 •
edited
Loading

Can you provide more context and/or links on what you mean by "binary 2a2513e8687993ed71cc48b09676abf18bb0e739f15473d36c50627b05036686" and "the buildid code"? Is this related to GOROOT/src/cmd/go/internal/work/buildid.go or something else?

@dmitshur dmitshur added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 17, 2023
@stevemk14ebr
Copy link
Contributor Author

stevemk14ebr commented Aug 17, 2023 •
edited
Loading

This code here https://cs.opensource.google/go/go/+/master:src/cmd/internal/buildid/buildid.go;drc=615c7c18a70e0d6638accdb0fcc5f60c57a2118b;l=289 there is a bug in readELF (https://cs.opensource.google/go/go/+/master:src/cmd/internal/buildid/note.go;drc=14a3ffc3d2165ef03c3ffd0037a4fa6dbb776026;l=154) as I commented above.

https://www.virustotal.com/gui/file/2a2513e8687993ed71cc48b09676abf18bb0e739f15473d36c50627b05036686 sha256 hashes are commonly used to refer to files on VT.

@dmitshur dmitshur changed the title affected/package: buildid invalid read when program alignment is zero cmd/internal/buildid: buildid invalid read when program alignment is zero Aug 17, 2023
@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Aug 17, 2023
@dmitshur dmitshur added this to the Backlog milestone Aug 17, 2023
@dmitshur
Copy link
Contributor

CC @bcmills, @rsc.

@ianlancetaylor
Copy link
Member

It's pretty strange to have a PT_NOTE segment with a p_align field of 0, but I guess we should handle it. I'll send a CL.

@bcmills bcmills added the NeedsFix The path to resolution is known, but the work has not been done. label Aug 17, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 17, 2023
@stevemk14ebr
Copy link
Contributor Author

I agree, I had to lookup if alignment of 0 was even valid. Apparently it is.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/520597 mentions this issue: cmd/internal/buildid: don't crash on 0 phdr.p_align field

@dmitshur dmitshur modified the milestones: Backlog, Go1.22 Aug 17, 2023
@golang golang locked and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.22
Development

No branches or pull requests
5 participants
@dmitshur @ianlancetaylor @bcmills @stevemk14ebr @gopherbot