Skip to content

Commit

Permalink
:fix: toctou for files
Browse files Browse the repository at this point in the history
- Fixes #440

Signed-off-by: naveensrinivasan <172697+naveensrinivasan@users.noreply.github.com>
  • Loading branch information
naveensrinivasan committed Feb 21, 2024
1 parent ef4b577 commit b2f2c5a
Show file tree
Hide file tree
Showing 4 changed files with 122 additions and 3 deletions.
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")
}
}

0 comments on commit b2f2c5a

Please sign in to comment.