Path Traversal Vulnerability in extractPkgFromBundle

[naveensrinivasan]

Environment

Device and OS:
App version:
Kubernetes distro being used:
Other:

Expected Result

File paths should be sanitized to prevent extraction outside the specified target directory.

Actual Result

The extractPkgFromBundle function does not currently sanitize paths, which could allow for path traversal, enabling files to be extracted to arbitrary locations on the filesystem.

Visual Proof (screenshots, videos, text, etc)

N/A

Severity/Priority

High - This vulnerability could lead to overwriting critical files or placing malicious files in sensitive directories, potentially compromising the system's integrity or security.

Additional Context

Issue Details

The extractPkgFromBundlefunction in the application extracts files from a tarball without sanitizing their paths. This lack of sanitization could allow an attacker to craft a tarball that, when processed, places files outside of the intended directory. Such a vulnerability is particularly concerning because it could lead to unauthorized access, information disclosure, or execution of malicious code.

How to Fix

To mitigate this vulnerability, the application should implement path sanitization within the extractPkgFromBundle function. Specifically:

import (
    "fmt"
    "os"
    "path/filepath"
    "strings"
)

// extractPkgFromBundle extracts a Zarf package from a local tarball bundle
func (t *TarballBundle) extractPkgFromBundle() ([]string, error) {
    ...
    extractLayer := func(_ context.Context, file av4.File) error {
        ...
        // Clean the path to remove any ../ or similar sequences
        cleanPath := filepath.Clean(desc.Annotations[ocispec.AnnotationTitle])

        // Ensure the cleaned path is within the intended directory
        if !strings.HasPrefix(cleanPath, filepath.Clean(t.TmpDir)+string(os.PathSeparator)) {
            return fmt.Errorf("attempt to write outside of target directory: %s", cleanPath)
        }

        layerDst := filepath.Join(t.TmpDir, cleanPath)
        ...
    }
    ...
}

This code snippet demonstrates how to sanitize file paths by cleaning them and validating that they do not attempt to traverse outside the intended directory. The filepath.Clean function is used to normalize the path, and strings.HasPrefix ensures the path is within the t.TmpDir directory.

Further Reading

You can refer to the OWASP (Open Web Application Security Project) guidelines for more information on path traversal vulnerabilities and how they can be mitigated. OWASP provides a wealth of resources on web security, including path traversal:
https://www.stackhawk.com/blog/golang-path-traversal-guide-examples-and-prevention/
OWASP Path Traversal

[naveensrinivasan]

This is applicable to a few other areas in code.

[naveensrinivasan]

We can implement a straightforward checksum verification process to address the need for checksum validation of downloaded layers. This ensures the integrity of each layer by comparing its calculated SHA256 checksum against the expected checksum provided in the OCI descriptor.

First, we introduce a utility function ValidateChecksum to calculate and compare the SHA256 checksum of a file:

// ValidateChecksum compares the SHA256 checksum of the specified file with the expected checksum.
func ValidateChecksum(filePath string, expectedChecksum string) (bool, error) {
	file, err := os.Open(filePath)
	if err != nil {
		return false, fmt.Errorf("failed to open file %s for checksum validation: %w", filePath, err)
	}
	defer file.Close()

	hasher := sha256.New()
	if _, err := io.Copy(hasher, file); err != nil {
		return false, fmt.Errorf("failed to calculate checksum for file %s: %w", filePath, err)
	}

	actualChecksum := hex.EncodeToString(hasher.Sum(nil))
	return actualChecksum == expectedChecksum, nil
}

Next, we modify the downloadPkgFromRemoteBundle method in src/pkg/sources/remote.go to utilize this function after a layer is downloaded. This ensures the downloaded layer's integrity by validating its checksum:

// Inside the downloadPkgFromRemoteBundle method, after downloading a layer
// Assume `dst` is the path to the downloaded layer file

// Validate checksum after downloading
expectedChecksum := layer.Digest.Encoded() // Extract the expected checksum from the layer's digest
valid, err := utils.ValidateChecksum(dst, expectedChecksum)
if err != nil {
    return nil, fmt.Errorf("error validating checksum for layer %s: %v", layer.Digest, err)
}
if !valid {
    return nil, fmt.Errorf("checksum validation failed for layer %s", layer.Digest)
}

This implementation calculates the SHA256 checksum of the downloaded file and compares it with the expected checksum. If the checksums do not match, it indicates potential tampering or corruption, and an error is returned. This approach enhances the security and integrity of the download process in RemoteBundle.

not tested

[UncleGedd]

Great catch! Will prioritize and fix