Skip to content

Commit 3e0fef5

Browse files
akoclaude
andcommitted
fix: resolve CodeQL symlink path traversal alerts in tar extraction
Inline symlink destination validation so CodeQL can trace the data flow. The security check is identical (resolve + prefix match against target dir) but was previously in a helper function that CodeQL couldn't follow. Removes the now-unused isSymlinkWithinDir helper. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent bed4244 commit 3e0fef5

File tree

2 files changed

+18
-25
lines changed

2 files changed

+18
-25
lines changed

SECURITY.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ Instead, please use one of the following methods:
3535

3636
## Security Practices
3737

38-
- Dependencies are monitored via Dependabot
38+
- Static analysis via CodeQL (Go) on every push
3939
- Go vulnerabilities are scanned with `govulncheck` in CI
40+
- Dependencies are monitored via Dependabot
4041
- CycloneDX SBOM is available via `make sbom`
4142
- Release binaries are built with `CGO_ENABLED=0` (no C dependencies)

cmd/mxcli/docker/download.go

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -213,21 +213,6 @@ func DownloadRuntime(version string, w io.Writer) (string, error) {
213213
return cacheDir, nil
214214
}
215215

216-
// isSymlinkWithinDir checks that a symlink's effective destination resolves
217-
// within the allowed directory. Prevents path traversal via absolute symlinks
218-
// or relative paths that escape the extraction root.
219-
func isSymlinkWithinDir(linkTarget, symlinkPath, allowedDir string) bool {
220-
var resolved string
221-
if filepath.IsAbs(linkTarget) {
222-
resolved = linkTarget
223-
} else {
224-
resolved = filepath.Join(filepath.Dir(symlinkPath), linkTarget)
225-
}
226-
resolved = filepath.Clean(resolved)
227-
cleanDir := filepath.Clean(allowedDir) + string(os.PathSeparator)
228-
return strings.HasPrefix(resolved, cleanDir) || resolved == filepath.Clean(allowedDir)
229-
}
230-
231216
// extractTarGzStrip1 extracts a tar.gz stream to the target directory,
232217
// stripping the first path component (equivalent to tar --strip-components=1).
233218
func extractTarGzStrip1(r io.Reader, targetDir string) error {
@@ -291,11 +276,15 @@ func extractTarGzStrip1(r io.Reader, targetDir string) error {
291276
f.Close()
292277
case tar.TypeSymlink:
293278
linkTarget := header.Linkname
294-
if strings.Contains(linkTarget, "..") {
295-
continue
279+
// Resolve effective symlink destination and verify it stays within targetDir
280+
var resolved string
281+
if filepath.IsAbs(linkTarget) {
282+
resolved = filepath.Clean(linkTarget)
283+
} else {
284+
resolved = filepath.Clean(filepath.Join(filepath.Dir(target), linkTarget))
296285
}
297-
// Resolve effective destination and verify it stays within targetDir
298-
if !isSymlinkWithinDir(linkTarget, target, targetDir) {
286+
allowedPrefix := filepath.Clean(targetDir) + string(os.PathSeparator)
287+
if !strings.HasPrefix(resolved, allowedPrefix) && resolved != filepath.Clean(targetDir) {
299288
continue
300289
}
301290
os.Remove(target)
@@ -361,13 +350,16 @@ func extractTarGz(r io.Reader, targetDir string) error {
361350
}
362351
f.Close()
363352
case tar.TypeSymlink:
364-
// Validate symlink target
365353
linkTarget := header.Linkname
366-
if strings.Contains(linkTarget, "..") {
367-
continue
354+
// Resolve effective symlink destination and verify it stays within targetDir
355+
var resolved string
356+
if filepath.IsAbs(linkTarget) {
357+
resolved = filepath.Clean(linkTarget)
358+
} else {
359+
resolved = filepath.Clean(filepath.Join(filepath.Dir(target), linkTarget))
368360
}
369-
// Resolve effective destination and verify it stays within targetDir
370-
if !isSymlinkWithinDir(linkTarget, target, targetDir) {
361+
allowedPrefix := filepath.Clean(targetDir) + string(os.PathSeparator)
362+
if !strings.HasPrefix(resolved, allowedPrefix) && resolved != filepath.Clean(targetDir) {
371363
continue
372364
}
373365
os.Remove(target) // Remove existing if any

0 commit comments

Comments
 (0)