From c62dc82988e12352ca888a3f97545d1617bbe321 Mon Sep 17 00:00:00 2001 From: Stefan Prodan Date: Fri, 29 Sep 2023 11:03:49 +0300 Subject: [PATCH] Skip symlinks during OCI layer decompression Signed-off-by: Stefan Prodan --- http/fetch/archive_fetcher.go | 2 +- http/fetch/go.mod | 2 +- http/fetch/go.sum | 4 +- oci/client/pull.go | 5 +- oci/go.mod | 2 +- oci/go.sum | 4 +- tar/go.mod | 2 +- tar/go.sum | 4 +- tar/symlink_test.go | 128 ++++++++++++++++++++++++++++++++++ tar/tar.go | 7 ++ tar/tar_opts.go | 7 ++ 11 files changed, 155 insertions(+), 12 deletions(-) create mode 100644 tar/symlink_test.go diff --git a/http/fetch/archive_fetcher.go b/http/fetch/archive_fetcher.go index 584c0269..50f33c39 100644 --- a/http/fetch/archive_fetcher.go +++ b/http/fetch/archive_fetcher.go @@ -140,7 +140,7 @@ func (r *ArchiveFetcher) Fetch(archiveURL, digest, dir string) error { } // Extracts the tar file. - if err = tar.Untar(f, dir, tar.WithMaxUntarSize(r.maxUntarSize)); err != nil { + if err = tar.Untar(f, dir, tar.WithMaxUntarSize(r.maxUntarSize), tar.WithSkipSymlinks()); err != nil { return fmt.Errorf("failed to extract archive (check whether file size exceeds max download size): %w", err) } diff --git a/http/fetch/go.mod b/http/fetch/go.mod index d1514998..cbc1e870 100644 --- a/http/fetch/go.mod +++ b/http/fetch/go.mod @@ -21,7 +21,7 @@ require ( ) require ( - github.com/cyphar/filepath-securejoin v0.2.3 // indirect + github.com/cyphar/filepath-securejoin v0.2.4 // indirect github.com/google/go-cmp v0.5.9 // indirect github.com/hashicorp/go-cleanhttp v0.5.2 // indirect github.com/zeebo/blake3 v0.1.1 // indirect diff --git a/http/fetch/go.sum b/http/fetch/go.sum index 442e008d..38dbe2ca 100644 --- a/http/fetch/go.sum +++ b/http/fetch/go.sum @@ -1,5 +1,5 @@ -github.com/cyphar/filepath-securejoin v0.2.3 h1:YX6ebbZCZP7VkM3scTTokDgBL2TY741X51MTk3ycuNI= -github.com/cyphar/filepath-securejoin v0.2.3/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= +github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= +github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/go-logr/logr v1.2.4 h1:g01GSCwiDw2xSZfjJ2/T9M+S6pFdcNtFYsp+Y43HYDQ= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= diff --git a/oci/client/pull.go b/oci/client/pull.go index d4828687..b23452c2 100644 --- a/oci/client/pull.go +++ b/oci/client/pull.go @@ -20,9 +20,10 @@ import ( "context" "fmt" - "github.com/fluxcd/pkg/tar" "github.com/google/go-containerregistry/pkg/crane" "github.com/google/go-containerregistry/pkg/name" + + "github.com/fluxcd/pkg/tar" ) // Pull downloads an artifact from an OCI repository and extracts the content to the given directory. @@ -65,7 +66,7 @@ func (c *Client) Pull(ctx context.Context, url, outDir string) (*Metadata, error return nil, fmt.Errorf("extracting first layer failed: %w", err) } - if err = tar.Untar(blob, outDir, tar.WithMaxUntarSize(-1)); err != nil { + if err = tar.Untar(blob, outDir, tar.WithMaxUntarSize(-1), tar.WithSkipSymlinks()); err != nil { return nil, fmt.Errorf("failed to untar first layer: %w", err) } diff --git a/oci/go.mod b/oci/go.mod index fc18183d..3c789336 100644 --- a/oci/go.mod +++ b/oci/go.mod @@ -48,7 +48,7 @@ require ( github.com/bugsnag/panicwrap v0.0.0-20151223152923-e2c28503fcd0 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/containerd/stargz-snapshotter/estargz v0.14.3 // indirect - github.com/cyphar/filepath-securejoin v0.2.3 // indirect + github.com/cyphar/filepath-securejoin v0.2.4 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/docker/cli v24.0.0+incompatible // indirect github.com/docker/distribution v2.8.2+incompatible // indirect diff --git a/oci/go.sum b/oci/go.sum index 6a6c4214..6e411fee 100644 --- a/oci/go.sum +++ b/oci/go.sum @@ -72,8 +72,8 @@ github.com/containerd/stargz-snapshotter/estargz v0.14.3 h1:OqlDCK3ZVUO6C3B/5FSk github.com/containerd/stargz-snapshotter/estargz v0.14.3/go.mod h1:KY//uOCIkSuNAHhJogcZtrNHdKrA99/FCCRjE3HD36o= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/cyphar/filepath-securejoin v0.2.3 h1:YX6ebbZCZP7VkM3scTTokDgBL2TY741X51MTk3ycuNI= -github.com/cyphar/filepath-securejoin v0.2.3/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= +github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= +github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= diff --git a/tar/go.mod b/tar/go.mod index 7cd0a8e7..c507ee8d 100644 --- a/tar/go.mod +++ b/tar/go.mod @@ -2,4 +2,4 @@ module github.com/fluxcd/pkg/tar go 1.20 -require github.com/cyphar/filepath-securejoin v0.2.3 +require github.com/cyphar/filepath-securejoin v0.2.4 diff --git a/tar/go.sum b/tar/go.sum index 2c534903..de447c23 100644 --- a/tar/go.sum +++ b/tar/go.sum @@ -1,2 +1,2 @@ -github.com/cyphar/filepath-securejoin v0.2.3 h1:YX6ebbZCZP7VkM3scTTokDgBL2TY741X51MTk3ycuNI= -github.com/cyphar/filepath-securejoin v0.2.3/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= +github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= +github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= diff --git a/tar/symlink_test.go b/tar/symlink_test.go new file mode 100644 index 00000000..e8c4905f --- /dev/null +++ b/tar/symlink_test.go @@ -0,0 +1,128 @@ +/* +Copyright 2023 The Flux 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 tar + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "io" + "os" + "path" + "path/filepath" + "testing" +) + +func TestSkipSymlinks(t *testing.T) { + tmpDir := t.TempDir() + + symlinkTarget := filepath.Join(tmpDir, "symlink.target") + err := os.WriteFile(symlinkTarget, geRandomContent(256), os.ModePerm) + if err != nil { + t.Fatal(err) + } + + symlink := filepath.Join(tmpDir, "symlink") + err = os.Symlink(symlinkTarget, symlink) + if err != nil { + t.Fatal(err) + } + + tgzFileName := filepath.Join(t.TempDir(), "test.tgz") + var buf bytes.Buffer + err = tgzWithSymlinks(tmpDir, &buf) + if err != nil { + t.Fatal(err) + } + + tgzFile, err := os.OpenFile(tgzFileName, os.O_CREATE|os.O_RDWR, os.ModePerm) + if err != nil { + t.Fatal(err) + } + if _, err := io.Copy(tgzFile, &buf); err != nil { + t.Fatal(err) + } + if err = tgzFile.Close(); err != nil { + t.Fatal(err) + } + + targetDirOutput := filepath.Join(t.TempDir(), "output") + f1, err := os.Open(tgzFileName) + if err != nil { + t.Fatal(err) + } + + err = Untar(f1, targetDirOutput, WithMaxUntarSize(-1)) + if err == nil { + t.Errorf("wanted error: unsupported symlink") + } + + f2, err := os.Open(tgzFileName) + if err != nil { + t.Fatal(err) + } + + err = Untar(f2, targetDirOutput, WithMaxUntarSize(-1), WithSkipSymlinks()) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + if _, err := os.Open(path.Join(targetDirOutput, "symlink.target")); err != nil { + t.Errorf("regular file not found: %v", err) + } +} + +func tgzWithSymlinks(src string, buf io.Writer) error { + absDir, err := filepath.Abs(src) + if err != nil { + return err + } + + zr := gzip.NewWriter(buf) + tw := tar.NewWriter(zr) + if err := filepath.Walk(absDir, func(file string, fi os.FileInfo, err error) error { + header, err := tar.FileInfoHeader(fi, file) + if err != nil { + return err + } + if err := tw.WriteHeader(header); err != nil { + return err + } + + if fi.Mode().IsRegular() { + f, err := os.Open(file) + if err != nil { + return err + } + if _, err := io.Copy(tw, f); err != nil { + return err + } + return f.Close() + } + + return nil + }); err != nil { + return err + } + if err := tw.Close(); err != nil { + return err + } + if err := zr.Close(); err != nil { + return err + } + return nil +} diff --git a/tar/tar.go b/tar/tar.go index 9556c23a..1ee8cd6c 100644 --- a/tar/tar.go +++ b/tar/tar.go @@ -39,6 +39,9 @@ type tarOpts struct { // maxUntarSize represents the limit size (bytes) for archives being decompressed by Untar. // When max is a negative value the size checks are disabled. maxUntarSize int + + // skipSymlinks ignores symlinks instead of failing the decompression. + skipSymlinks bool } // Untar reads the gzip-compressed tar file from r and writes it into dir. @@ -168,6 +171,10 @@ func Untar(r io.Reader, dir string, inOpts ...TarOption) (err error) { return err } madeDir[abs] = true + case mode&os.ModeSymlink == os.ModeSymlink: + if !opts.skipSymlinks { + return fmt.Errorf("tar file entry %s is a symlink, which is not allowed in this context", f.Name) + } default: return fmt.Errorf("tar file entry %s contained unsupported file type %v", f.Name, mode) } diff --git a/tar/tar_opts.go b/tar/tar_opts.go index eb8dfdc4..127dfdfe 100644 --- a/tar/tar_opts.go +++ b/tar/tar_opts.go @@ -27,6 +27,13 @@ func WithMaxUntarSize(max int) TarOption { } } +// WithSkipSymlinks allows for symlinks to be present in the tarball and skips them when decompressing. +func WithSkipSymlinks() TarOption { + return func(t *tarOpts) { + t.skipSymlinks = true + } +} + func (t *tarOpts) applyOpts(tarOpts ...TarOption) { for _, clientOpt := range tarOpts { clientOpt(t)