Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 16 additions & 7 deletions pkg/apk/apk/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -493,12 +493,21 @@ func cachePathFromURL(root string, u url.URL) (string, error) {

// url encode it so it can be a single directory
repoDir = url.QueryEscape(u2.String())
cacheFile := filepath.Join(root, repoDir, dir, filename)
// validate it is within root
cacheFile = filepath.Clean(cacheFile)
cleanroot := filepath.Clean(root)
if !strings.HasPrefix(cacheFile, cleanroot) {
return "", fmt.Errorf("cache file %s is not within root %s", cacheFile, cleanroot)
relativePath := filepath.Join(repoDir, dir, filename)

// Use os.Root for secure path validation to prevent path traversal

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look out for other calls to filepath.Join here to see whether or not they are similarly vulnerable - https://github.com/chainguard-dev/apko/pull/1795/files#diff-085f287e1519fcab77329f1fededeae7b771c1e18f0522e4c4f1fdbe3c202d3bR376 looks suspicious.

rootFS, err := os.OpenRoot(root)
if err != nil {
return "", fmt.Errorf("failed to open root directory %q: %w", root, err)
}
return cacheFile, nil
defer rootFS.Close()

// Validate that the relative path is safe and within the root
// This prevents path traversal attacks including symlink attacks
if _, err := rootFS.Stat(relativePath); err != nil && !os.IsNotExist(err) {
return "", fmt.Errorf("invalid cache path %q: %w", relativePath, err)
}

// Return the absolute path for use by other functions
return filepath.Join(root, relativePath), nil

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"root" isn't necessarily an absolute path; if this comment is to be accurate, you should use filepath.Abs.

}
Loading