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 1 commit
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
18 changes: 16 additions & 2 deletions src/pkg/bundle/tarball.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,11 @@ func (tp *tarballBundleProvider) getBundleManifest() error {
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")
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 +139,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 Down Expand Up @@ -169,6 +178,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
28 changes: 28 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,29 @@ 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
}
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")
}
}