Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: toctou for files #443

Merged
merged 4 commits into from
Feb 23, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
github.com/AlecAivazis/survey/v2 v2.3.7
github.com/alecthomas/jsonschema v0.0.0-20220216202328-9eeeec9d044b
github.com/defenseunicorns/zarf v0.32.3
github.com/fsnotify/fsnotify v1.7.0
github.com/goccy/go-yaml v1.11.3
github.com/mholt/archiver/v3 v3.5.1
github.com/mholt/archiver/v4 v4.0.0-alpha.8
Expand Down Expand Up @@ -197,7 +198,6 @@ require (
github.com/fluxcd/pkg/apis/kustomize v1.3.0 // indirect
github.com/fluxcd/pkg/apis/meta v1.3.0 // indirect
github.com/fluxcd/source-controller/api v1.2.4 // indirect
github.com/fsnotify/fsnotify v1.7.0 // indirect
github.com/fvbommel/sortorder v1.1.0 // indirect
github.com/gabriel-vasile/mimetype v1.4.3 // indirect
github.com/gdamore/encoding v1.0.0 // indirect
Expand Down
30 changes: 25 additions & 5 deletions src/pkg/bundle/tarball.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func (tp *tarballBundleProvider) CreateBundleSBOM(extractSBOM bool) error {
return err
}
// make tmp dir for pkg SBOM extraction
err = os.Mkdir(filepath.Join(tp.dst, config.BundleSBOM), 0700)
err = os.Mkdir(filepath.Join(tp.dst, config.BundleSBOM), 0o700)
if err != nil {
return err
}
Expand Down Expand Up @@ -117,12 +117,21 @@ func (tp *tarballBundleProvider) getBundleManifest() error {
if tp.bundleRootManifest != nil {
return nil
}
// Create a secure temporary directory for handling files
secureTempDir, err := utils.CreateSecureTempDir()
if err != nil {
return fmt.Errorf("failed to create a secure temporary directory: %w", err)
}
defer os.RemoveAll(secureTempDir) // Ensure cleanup of the temp directory

if err := av3.Extract(tp.src, "index.json", tp.dst); err != nil {
return fmt.Errorf("failed to extract index.json from %s: %w", tp.src, err)
}

indexPath := filepath.Join(tp.dst, "index.json")
indexPath := filepath.Join(secureTempDir, "index.json")
indexPathChecksum, err := utils.CalculateFileChecksum(indexPath)
if err != nil {
return fmt.Errorf("failed to calculate checksum for %s: %w", indexPath, err)
}

defer os.Remove(indexPath)

Expand All @@ -136,7 +145,13 @@ func (tp *tarballBundleProvider) getBundleManifest() error {
if err := json.Unmarshal(b, &index); err != nil {
return err
}

// The check is done after json.Unmarshal to ensure that the file wasn't tampered after written to the temp dir
// while reading the file
// Verify the calculated checksum against the expected checksum
isValid, err := utils.VerifyFileChecksum(indexPath, indexPathChecksum)
if err != nil || !isValid {
return fmt.Errorf("checksum verification failed for %s", indexPath)
}
// local bundles only have one manifest entry in their index.json
bundleManifestDesc := index.Manifests[0]
tp.bundleRootDesc = bundleManifestDesc
Expand All @@ -151,7 +166,7 @@ func (tp *tarballBundleProvider) getBundleManifest() error {
return fmt.Errorf("failed to extract %s from %s: %w", bundleManifestDesc.Digest.Encoded(), tp.src, err)
}

manifestPath := filepath.Join(tp.dst, manifestRelativePath)
manifestPath := filepath.Join(secureTempDir, manifestRelativePath)

defer os.Remove(manifestPath)

Expand All @@ -169,6 +184,11 @@ func (tp *tarballBundleProvider) getBundleManifest() error {
if err := json.Unmarshal(b, &manifest); err != nil {
return err
}
// This is to ensure that the file wasn't tampered after written to the temp dir
isValid, err = utils.VerifyFileChecksum(manifestPath, bundleManifestDesc.Digest.Encoded())
if err != nil || !isValid {
return fmt.Errorf("checksum verification failed for %s", manifestPath)
}

tp.bundleRootManifest = manifest
return nil
Expand Down
44 changes: 44 additions & 0 deletions src/pkg/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package utils

import (
"context"
"crypto/sha256"
"encoding/hex"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -120,3 +122,45 @@ func ToLocalFile(t any, filePath string) error {
func IsRemotePkg(pkg types.Package) bool {
return pkg.Repository != ""
}

// CalculateFileChecksum calculates the SHA-256 checksum of a file.
func CalculateFileChecksum(filePath string) (string, error) {
file, err := os.Open(filePath)
if err != nil {
return "", fmt.Errorf("error opening file: %w", err)
}
defer file.Close()

hasher := sha256.New()
if _, err := io.Copy(hasher, file); err != nil {
return "", fmt.Errorf("error calculating checksum: %w", err)
}

return hex.EncodeToString(hasher.Sum(nil)), nil
}

// VerifyFileChecksum compares the calculated checksum of a file against a provided checksum.
func VerifyFileChecksum(filePath, expectedChecksum string) (bool, error) {
calculatedChecksum, err := CalculateFileChecksum(filePath)
if err != nil {
return false, fmt.Errorf("error calculating checksum: %w", err)
}

return calculatedChecksum == expectedChecksum, nil
}

// CreateSecureTempDir creates a secure, randomly named subdirectory with restricted permissions.
func CreateSecureTempDir() (string, error) {
// Restrict permissions to the directory
// the permissions are being restricted to 0700 for security reasons
// create a subdirectory with a random name
secureTempDir, err := os.MkdirTemp(os.TempDir(), "uds-*")
if err != nil {
return "", fmt.Errorf("failed to create a secure temp directory: %w", err)
}
if err := os.Chmod(secureTempDir, 0o700); err != nil {
os.Remove(secureTempDir) // Cleanup on error
return "", fmt.Errorf("failed to set permissions for the temp directory: %w", err)
}
return secureTempDir, nil
}
77 changes: 77 additions & 0 deletions src/pkg/utils/utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// SPDX-License-Identifier: Apache-2.0
// SPDX-FileCopyrightText: 2023-Present The UDS Authors

// Package utils provides utility fns for UDS-CLI
package utils

import (
"os"
"testing"
)

func TestCalculateFileChecksum(t *testing.T) {
// Create a temporary file
tmpFile, err := os.CreateTemp("", "testfile-")
if err != nil {
t.Fatalf("Unable to create temporary file: %v", err)
}
defer os.Remove(tmpFile.Name()) // clean up

// Write some content to the file
content := []byte("hello world")
if _, err := tmpFile.Write(content); err != nil {
t.Fatalf("Unable to write to temporary file: %v", err)
}
tmpFile.Close()

// Expected checksum of "hello world"
expectedChecksum := "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9"

checksum, err := CalculateFileChecksum(tmpFile.Name())
if err != nil {
t.Errorf("CalculateFileChecksum returned an error: %v", err)
}

if checksum != expectedChecksum {
t.Errorf("Expected checksum %s, got %s", expectedChecksum, checksum)
}
}

func TestVerifyFileChecksum(t *testing.T) {
// Create a temporary file
tmpFile, err := os.CreateTemp("", "testfile-")
if err != nil {
t.Fatalf("Unable to create temporary file: %v", err)
}
defer os.Remove(tmpFile.Name()) // clean up

// Write some content to the file
content := []byte("hello world")
if _, err := tmpFile.Write(content); err != nil {
t.Fatalf("Unable to write to temporary file: %v", err)
}
tmpFile.Close()

// Expected checksum of "hello world"
expectedChecksum := "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9"

valid, err := VerifyFileChecksum(tmpFile.Name(), expectedChecksum)
if err != nil {
t.Errorf("VerifyFileChecksum returned an error: %v", err)
}

if !valid {
t.Errorf("Expected checksum verification to be true, got false")
}

// Test with incorrect checksum
invalidChecksum := "invalidchecksum"
valid, err = VerifyFileChecksum(tmpFile.Name(), invalidChecksum)
if err != nil {
t.Errorf("Didn't expect an error for incorrect checksum, got %v", err)
}

if valid {
t.Errorf("Expected checksum verification to be false, got true")
}
}