pkg/v1/mutate: preserve relative symlinks that stay within rootfs in Extract#2279
Conversation
…Extract The `extract()` function in `pkg/v1/mutate/mutate.go` had all symlink filtering removed in the google#2250 revert. Without any filtering, images can contain symlinks with absolute targets (e.g. `/etc/passwd`) or relative targets that escape the rootfs (e.g. `../../../tmp/evil`), both of which are unsafe when layers are extracted to disk. Signed-off-by: anish k <ak8686@princeton.edu>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2279 +/- ##
==========================================
+ Coverage 56.74% 56.76% +0.01%
==========================================
Files 165 165
Lines 11248 11253 +5
==========================================
+ Hits 6383 6388 +5
Misses 4106 4106
Partials 759 759 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…false positive The filepath.Join at this line is only used to compute a resolved path for validation purposes; no file I/O is performed with it. Gosec G305 fires because the path is constructed from user-supplied tar header fields, but the guard logic immediately after is exactly what prevents any traversal. Signed-off-by: anish k <ak8686@princeton.edu>
| if filepath.IsAbs(header.Linkname) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
This code is called for both crane export and crane flatten. #2238 shows us a use-case for when absolute links should be preserved. So, there is still open discussion on whether we should prune absolute links at all. I suggest for now skipping this check on absolute paths,
There was a problem hiding this comment.
Good point, removed the absolute-path check. Now only relative targets that resolve outside the rootfs are dropped; absolute targets pass through unchanged. Updated the test and comment to match, referencing #2238.
…#2238 discussion Signed-off-by: anish k <ak8686@princeton.edu>
|
Whoops, I tried manually merging on the UI and messed up the formatting. Could you re-merge for submit? Sorry about that. |
…Extract (google#2279) * pkg/v1/mutate: preserve relative symlinks that stay within rootfs in Extract The `extract()` function in `pkg/v1/mutate/mutate.go` had all symlink filtering removed in the google#2250 revert. Without any filtering, images can contain symlinks with absolute targets (e.g. `/etc/passwd`) or relative targets that escape the rootfs (e.g. `../../../tmp/evil`), both of which are unsafe when layers are extracted to disk. Signed-off-by: anish k <ak8686@princeton.edu>
The
extract()function inpkg/v1/mutate/mutate.gohad all symlink filtering removed in the #2250 revert. Without any filtering, images can contain symlinks with absolute targets (e.g./etc/passwd) or relative targets that escape the rootfs (e.g.../../../tmp/evil), both of which are unsafe when layers are extracted to disk.At the same time, #2268 reports that a prior version of the filtering was over-aggressive: it dropped all relative symlinks that contained
..anywhere in their target, even safe ones like../lib/ld-linux-x86-64.so.2fromusr/local/bin/— a pattern common in glibc and C toolchains.This change adds targeted filtering back in
extract(): absoluteLinknamevalues are always dropped, and relative ones are resolved against the symlink's own directory usingfilepath.Join+filepath.Clean; only those whose resolved path starts with..(i.e. would leave the rootfs) are dropped. Safe relative symlinks that resolve inside the rootfs are preserved unchanged.A new table-driven test
TestExtractSymlinkFilteringcovers the four cases: safe relative symlink that stays inside rootfs (preserved), another common glibc pattern (preserved), absolute target (dropped), and an escaping relative target (dropped).Fixes #2268