fix(golang): account for UPX's loader padding when unpacking Go binaries - #5347
Merged
Merged
Conversation
UPX pads its output to a 4 byte boundary before it writes the loader stub, and l_lsize counts from that boundary. The block chain resumed at the end of the last PT_LOAD extent plus l_lsize, so whenever the compressed extents ended off a 4 byte boundary it landed 1 to 3 bytes early, read a misaligned b_info, and stopped. The padding between the segments was never filled in, the reconstruction ended at that first hole, and .go.buildinfo in the data segment went with it, so the binary contributed no packages. The loader skip now rounds up to 4 before adding l_lsize, which matches the funpad4 in UPX's own unpacker. The crafted fixture for the tail extents put its stub straight after the head extent, the layout the bug assumed, so it now pads the way UPX does. Signed-off-by: huuyafwww <huuya.yamauchi@3-shake.com>
willmurphyscode
approved these changes
Sep 29, 2026
4 tasks done
willmurphyscode
added a commit
that referenced
this pull request
Sep 29, 2026
UPX pads to a 4 byte boundary before it writes the loader stub. The image-small-upx build happened to need no padding, so the cataloger tests over it passed with or without the fix in #5347. - build the fixture with -X main.Version=1.0.11, which needs 2 bytes of padding, so the existing cataloger tests fail without the fix - add TestImageSmallUPXNeedsLoaderPadding, which fails if a change to the fixture's inputs means it no longer needs padding - run the crafted loader test on both sides of the boundary, so a skip that always rounds up is caught too Signed-off-by: Will Murphy <willmurphyscode@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Hi, and thanks for maintaining Syft. We use Syft to generate SBOMs for container images, and while testing the upgrade to v1.52.0 we found that most Go binaries packed with
upx --lzmano longer produce any Go packages, not evenstdlib. The scan still exits 0, and the syft JSON only records an unknown on the file:unable to read golang buildinfo: UPX reconstruction is incomplete: the chain ended after 766321 of the declared 2756755 bytes. v1.51.1 finds the packages in the same binaries.The chain breaks where it resumes after the loader stub.
loaderSkip.pastaddsl_lsizeto the offset where the last PT_LOAD extent ended, but UPX pads the output to a 4 byte boundary before it writes the stub (sz_pack2a = fpad4(fo, total_out)at the end ofPackLinuxElf64::pack2), andPackLinuxElf::pack3countsl_lsizefrom that boundary. When the compressed extents end off a 4 byte boundary, the chain resumes 1 to 3 bytes early, reads a misalignedb_info(sz_cprcame out as0x1C00000Efor one of ours), and ends there. The padding between the segments is never filled in, the reconstruction stops at that first hole, and.go.buildinfoin the data segment goes with it. This rounds the offset up to 4 before addingl_lsize. UPX's own unpacker has the matchingfunpad4(fi)inPackLinuxElf64::unpack, taggedMATCH01like thefpad4on the pack side, and the padding and thel_lsizearithmetic are the same in the sources of 3.91, 3.94, 3.96, 4.0.2, 4.2.4, 5.0.0 and 5.2.1.It doesn't depend on the Go version. Whether a binary is affected comes down to where the compressed extents in front of the stub happen to end, so it looks random: across 36 builds of the same small program (Go 1.23.2 to 1.26.0, each with six different
-Xstrings, packed with UPX 5.2.1--best --lzma), v1.52.0 found the packages in exactly the 10 whose extents ended on a 4 byte boundary. Theimage-small-upxfixture is one of the lucky ones, its extents end at0x148830, which is why the Docker-backed tests stayed green. The crafted fixture inTestDecompressUPX_TailExtentsArePlacedPastTheLoaderput its stub straight after a head extent ending at byte 109, so it had the same assumption baked in as the code.I changed that test to lay the fixture out the way UPX does, with the padding in front of the stub, and added a
requirethat the head extent really ends off a 4 byte boundary, so the test can't quietly stop covering this if the LZMA encoder output ever changes. It fails on main (the tail extent is never placed) and passes here.With real binaries, I packed 66 files (Go 1.23.2 to 1.27.1, UPX 4.2.4 and 5.2.1, with
--best --lzma,--lzmaand--best --lzma --exact) and compared each reconstruction with the binary it was packed from. All 66 now come back byte for byte identical with no error, against 16 on main, andsyft scanfinds the main module,github.com/google/uuidandstdlibin all 66 (16 with v1.52.0, 66 with v1.51.1).go test ./syft/pkg/cataloger/golang/...including the Docker fixtures andmake static-analysispass with Go 1.26.8. I haven't runmake unit, the integration tests or the CLI tests locally.Happy to move the padding into the fixture helper, or to put it in a separate test if you'd rather leave the existing one as it was.
Type of change
Checklist
Issue references
Regression from #5195 (released in v1.52.0).