fix(utils): reject tar hardlinks that escape the extraction root - #11266
Merged
Conversation
Zelys-DFKH
force-pushed
the
fix/untar-hardlink-escape
branch
from
July 31, 2026 23:52
d22d415 to
cff3176
Compare
ExtractArchive pre-scans archive members and rejects symlinks, but tar hardlink entries carry a regular file mode and so pass that check. Header.Linkname was never validated, so an archive could create a link to a path outside the destination directory. Validate Linkname with the same path check already applied to member names. Hardlinks that resolve inside the extraction root still extract, so ordinary archives are unaffected. pkg/oci/image.go already resolves tar.TypeLink targets before using them; this brings the archive extraction path in line with it. Assisted-by: Claude:claude-opus-5 Signed-off-by: Zelys-DFKH <zelys@dfkhelper.com>
The existing hardlink test names a link target two levels above the extraction root, so its final assertion checked a path the link never resolved to and could not fail. Point the target one level up instead, at the path that assertion already names. Add two cases. The first uses a .tar.gz, where ExtractArchive binds a Tar config with OverwriteExisting set, and follows the link entry with a regular entry of the same name. Before the fix that pair linked to a file outside the root and then truncated it through the link, which the plain .tar case does not reach. The second extracts a hardlink whose target is an earlier member of the same archive, covering the claim that ordinary archives are unaffected. Assisted-by: Claude:claude-opus-5 Signed-off-by: Zelys-DFKH <zelys@dfkhelper.com>
Zelys-DFKH
force-pushed
the
fix/untar-hardlink-escape
branch
from
July 31, 2026 23:58
cff3176 to
9aa41dc
Compare
localai-org-maint-bot
approved these changes
Aug 1, 2026
localai-org-maint-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed the hardlink target validation and its interaction with mholt/archiver: tar hardlinks are resolved as filepath.Join(destination, Linkname), so applying the existing archive-member path validation to Linkname closes the escape while retaining in-root links. The focused archive suite passes locally (8/8), and go vet ./pkg/utils/..., gofmt, and git diff --check are clean. The unrelated full-package failures here were only missing ffmpeg and unavailable DNS. Good to merge. @mudler
mudler
enabled auto-merge (squash)
August 1, 2026 07:25
mudler
disabled auto-merge
August 1, 2026 07:25
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
ExtractArchiveinpkg/utils/untar.gowalks archive members before extracting and rejects two things: member paths that resolve outside the destination, and symlinks. Tar hardlink entries are covered by neither.Header.FileInfo()reports a regular file mode for a hardlink, so the symlink test does not match, andHeader.Linknameis never inspected. An archive can therefore name a link target outside the extraction directory and the extractor will create it.This passes
Linknamethrough the same validation already applied to member names, so a hardlink pointing outside the extraction root is rejected with the existing "unsafe path" error. Hardlinks whose target resolves inside the root still extract normally, so ordinary archives are unaffected.pkg/oci/image.goalready handles this on the OCI image path, resolvingtar.TypeLinktargets against the extraction root before using them. This brings the archive path in line with it.Notes for Reviewers
Three tests cover this, all in
pkg/utils/untar_test.go.rejects tar hardlinks that point outside the destinationwrites a tar holding a singletar.TypeLinkentry whoseLinknameis../escaped.txt, then asserts extraction fails and nothing lands outside the destination. Without the fix it fails, and the failure output shows the underlying archiver attempting to create the link outside the extraction directory.rejects tar hardlinks that overwrite a file outside the destinationuses a.tar.gz, which is whereExtractArchivebinds aTarconfig withOverwriteExistingset, and follows the link entry with a regular entry of the same name. Without the fix that pair links to a file above the extraction root and then truncates it through the link, soExtractArchivereturns nil and the outside file comes back holding the archive's contents instead of its own. The plain.tarcase never reaches the write.extracts tar hardlinks that stay inside the destinationlinks to an earlier member of the same archive and expects extraction to succeed, so the new check is not rejecting ordinary archives.The full
pkg/utilspackage passes on Linux with go1.26.0, 59 specs.gofmt -landgo vet ./pkg/utils/are clean.golangci-lintwould not run because the released binary is built with Go 1.25 while the module targets 1.26.0.Unrelated to this change and not touched here: the same package's suite hangs on Windows.
InTrustedRootinpkg/utils/path.goloopsfor path != "/", andfilepath.Diron a Windows volume root returns its input unchanged, so the loop never terminates. I mention it only to explain why I verified on Linux.Signed commits