Skip to content

Commit

Permalink
add CRDs archive verification to prevent CRDs archive extraction
Browse files Browse the repository at this point in the history
Signed-off-by: zhzhuang-zju <[email protected]>
  • Loading branch information
zhzhuang-zju committed Oct 18, 2024
1 parent dcdabc9 commit 61c1f12
Show file tree
Hide file tree
Showing 3 changed files with 217 additions and 0 deletions.
75 changes: 75 additions & 0 deletions operator/pkg/tasks/init/crd.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,14 @@ limitations under the License.
package tasks

import (
"archive/tar"
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"os"
"path"
"path/filepath"
"strings"

"k8s.io/klog/v2"
Expand All @@ -35,6 +37,7 @@ import (
var (
crdsFileSuffix = "crds.tar.gz"
crdPathSuffix = "crds"
crdsArchive = []string{"crds", "crds/bases", "crds/patches"}
)

// NewPrepareCrdsTask init a prepare-crds task
Expand All @@ -53,6 +56,10 @@ func NewPrepareCrdsTask() workflow.Task {
Name: "Unpack",
Run: runUnpack,
},
{
Name: "post-check",
Run: postCheck,
},
},
}
}
Expand Down Expand Up @@ -154,6 +161,9 @@ func runUnpack(r workflow.RunData) error {
exist, _ := util.PathExists(crdsPath)
if !exist {
klog.V(2).InfoS("[runUnpack] CRD yaml files do not exist, unpacking tar file", "unpackDir", crdsDir)
if err = util.CheckGzFiles(crdsTarPath, checkOperatorCrdsTar); err != nil {
return fmt.Errorf("[unpack] inValid crd tar, err: %w", err)
}
if err := util.Unpack(crdsTarPath, crdsDir); err != nil {
return fmt.Errorf("[unpack] failed to unpack crd tar, err: %w", err)
}
Expand All @@ -165,6 +175,29 @@ func runUnpack(r workflow.RunData) error {
return nil
}

func postCheck(r workflow.RunData) error {
data, ok := r.(InitData)
if !ok {
return errors.New("post-check task invoked with an invalid data struct")
}

crdsDir, err := getCrdsDir(data)
if err != nil {
return fmt.Errorf("[post-check] failed to get CRD dir, err: %w", err)
}

for _, archive := range crdsArchive {
expectedDir := filepath.Join(crdsDir, archive)
exist, _ := util.PathExists(expectedDir)
if !exist {
return fmt.Errorf("[post-check] Lacking the necessary file path: %s", expectedDir)
}
}

klog.V(2).InfoS("[post-check] Successfully post-check the crd tar archive", "karmada", klog.KObj(data))
return nil
}

func existCrdsTar(crdsDir string) bool {
files := util.ListFiles(crdsDir)
klog.V(2).InfoS("[existCrdsTar] Checking for CRD tar file in directory", "directory", crdsDir)
Expand All @@ -184,3 +217,45 @@ func getCrdsDir(data InitData) (string, error) {
hashedKey := hex.EncodeToString(hash[:])
return path.Join(data.DataDir(), "cache", hashedKey), nil
}

// checkOperatorCrdsTar checks if the CRDs package complies with file specifications.
// It verifies the following:
// 1. Whether the path is clean.
// 2. Whether the file directory structure meets expectations.
func checkOperatorCrdsTar(header *tar.Header) error {
switch header.Typeflag {
case tar.TypeDir:
// in Unix-like systems, directory paths in tar archives end with a slash (/) to distinguish them from file paths.
if strings.HasSuffix(header.Name, "/") && len(header.Name) >= 1 {
if !isCleanPath(header.Name[:len(header.Name)-1]) {
return fmt.Errorf("the given file contains unclean file dir: %s", header.Name)
}
}
if !isExpectedPath(header.Name, crdsArchive) {
return fmt.Errorf("the given file contains unexpected file dir: %s", header.Name)
}
case tar.TypeReg:
if !isCleanPath(header.Name) {
return fmt.Errorf("the given file contains unclean file path: %s", header.Name)
}
if !isExpectedPath(header.Name, crdsArchive) {
return fmt.Errorf("the given file contains unexpected file path: %s", header.Name)
}
default:
fmt.Printf("unknown type: %v in %s\n", header.Typeflag, header.Name)
}
return nil
}

func isExpectedPath(path string, expectedDirs []string) bool {
for _, dir := range expectedDirs {
if path == dir || strings.HasPrefix(path, dir+"/") {
return true
}
}
return false
}

func isCleanPath(path string) bool {
return path == filepath.Clean(path)
}
110 changes: 110 additions & 0 deletions operator/pkg/tasks/init/crd_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
/*
Copyright 2024 The Karmada Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package tasks

import (
"archive/tar"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
)

func TestCheckOperatorCrdsTar(t *testing.T) {
testItems := []struct {
name string
header *tar.Header
expectedErr error
}{
{
name: "unclean file dir 'crds/../'",
header: &tar.Header{
Name: "crds/../",
Typeflag: tar.TypeDir,
},
expectedErr: fmt.Errorf("the given file contains unclean file dir: %s", "crds/../"),
},
{
name: "unexpected file dir '../crds'",
header: &tar.Header{
Name: "../crds",
Typeflag: tar.TypeDir,
},
expectedErr: fmt.Errorf("the given file contains unexpected file dir: %s", "../crds"),
},
{
name: "unexpected file dir '..'",
header: &tar.Header{
Name: "..",
Typeflag: tar.TypeDir,
},
expectedErr: fmt.Errorf("the given file contains unexpected file dir: %s", ".."),
},
{
name: "expected file dir 'crds/'",
header: &tar.Header{
Name: "crds/",
Typeflag: tar.TypeDir,
},
expectedErr: nil,
},
{
name: "expected file dir 'crds'",
header: &tar.Header{
Name: "crds",
Typeflag: tar.TypeDir,
},
expectedErr: nil,
},
{
name: "unclean file path 'crds/../a.yaml'",
header: &tar.Header{
Name: "crds/../a.yaml",
Typeflag: tar.TypeReg,
},
expectedErr: fmt.Errorf("the given file contains unclean file path: %s", "crds/../a.yaml"),
},
{
name: "unexpected file path '../crds/a.yaml'",
header: &tar.Header{
Name: "../crds/a.yaml",
Typeflag: tar.TypeReg,
},
expectedErr: fmt.Errorf("the given file contains unexpected file path: %s", "../crds/a.yaml"),
},
{
name: "unexpected file path '../a.yaml'",
header: &tar.Header{
Name: "../a.yaml",
Typeflag: tar.TypeReg,
},
expectedErr: fmt.Errorf("the given file contains unexpected file path: %s", "../a.yaml"),
},
{
name: "expected file path 'crds/a.yaml'",
header: &tar.Header{
Name: "crds/a.yaml",
Typeflag: tar.TypeReg,
},
expectedErr: nil,
},

}
for _, item := range testItems {
assert.Equal(t, item.expectedErr, checkOperatorCrdsTar(item.header))
}
}
32 changes: 32 additions & 0 deletions operator/pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,38 @@ func Unpack(file, targetPath string) error {
return nil
}

// CheckGzFiles check if the given file meet the check function.
func CheckGzFiles(file string, check func(*tar.Header) error) error {
r, err := os.Open(file)
if err != nil {
return err
}
defer r.Close()

gr, err := gzip.NewReader(r)
if err != nil {
return fmt.Errorf("new reader failed. %v", err)
}
defer gr.Close()

tr := tar.NewReader(gr)
for {
header, err := tr.Next()
if err != nil {
if err == io.EOF {
break
}
return err
}

if err = check(header); err != nil {
return err
}
}

return nil
}

// ioCopyN fix Potential DoS vulnerability via decompression bomb.
func ioCopyN(outFile *os.File, tr *tar.Reader) error {
for {
Expand Down

0 comments on commit 61c1f12

Please sign in to comment.