Elgato_dark/argo-cd
1
0
Fork 0
mirror of https://github.com/argoproj/argo-cd synced 2026-04-21 17:07:16 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 22

fix: Ensure that symlink targets are not made absolute on extracting a tar (#24145) - backport/cherry-pick to 3.1 (#24519)

Browse source 
Signed-off-by: Linus Ehlers <Linus.Ehlers@ppi.de>
This commit is contained in:
Linus Ehlers 2025-09-11 16:48:02 +02:00 committed by GitHub
parent cfeed49105
commit 468870f65d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 98 additions and 36 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
test/e2e/fixture/repos/repos.go
View file

@ -212,7 +212,7 @@ func PushChartToOCIRegistry(t *testing.T, chartPathName, chartName, chartVersion
require.NoError(t, err1)
defer func() { _ = os.RemoveAll(tempDest) }()
chartAbsPath, err2 := filepath.Abs("./testdata/" + chartPathName)
chartAbsPath, err2 := filepath.Abs("./" + chartPathName)
require.NoError(t, err2)
t.Setenv("HELM_EXPERIMENTAL_OCI", "1")
@ -236,7 +236,7 @@ func PushChartToAuthenticatedOCIRegistry(t *testing.T, chartPathName, chartName,
require.NoError(t, err1)
defer func() { _ = os.RemoveAll(tempDest) }()
chartAbsPath, err2 := filepath.Abs("./testdata/" + chartPathName)
chartAbsPath, err2 := filepath.Abs("./" + chartPathName)
require.NoError(t, err2)
t.Setenv("HELM_EXPERIMENTAL_OCI", "1")
@ -274,13 +274,13 @@ func PushChartToAuthenticatedOCIRegistry(t *testing.T, chartPathName, chartName,
// PushImageToOCIRegistry adds a helm chart to helm OCI registry
func PushImageToOCIRegistry(t *testing.T, pathName, tag string) {
t.Helper()
imagePath := "./testdata/" + pathName
imagePath := "./" + pathName
errors.NewHandler(t).FailOnErr(fixture.Run(
imagePath,
"oras",
"push",
fmt.Sprintf("%s:%s", fmt.Sprintf("%s/%s", strings.TrimPrefix(fixture.OCIHostURL, "oci://"), pathName), tag),
fmt.Sprintf("%s:%s", fmt.Sprintf("%s/%s", strings.TrimPrefix(fixture.OCIHostURL, "oci://"), filepath.Base(pathName)), tag),
".",
))
}
@ -288,13 +288,13 @@ func PushImageToOCIRegistry(t *testing.T, pathName, tag string) {
// PushImageToAuthenticatedOCIRegistry adds a helm chart to helm OCI registry
func PushImageToAuthenticatedOCIRegistry(t *testing.T, pathName, tag string) {
t.Helper()
imagePath := "./testdata/" + pathName
imagePath := "./" + pathName
errors.NewHandler(t).FailOnErr(fixture.Run(
imagePath,
"oras",
"push",
fmt.Sprintf("%s:%s", fmt.Sprintf("%s/%s", strings.TrimPrefix(fixture.AuthenticatedOCIHostURL, "oci://"), pathName), tag),
fmt.Sprintf("%s:%s", fmt.Sprintf("%s/%s", strings.TrimPrefix(fixture.AuthenticatedOCIHostURL, "oci://"), filepath.Base(pathName)), tag),
".",
))
}

14
test/e2e/helm_test.go
View file

@ -552,7 +552,7 @@ func TestHelmRepoDiffLocal(t *testing.T) {
func TestHelmOCIRegistry(t *testing.T) {
Given(t).
PushChartToOCIRegistry("helm-values", "helm-values", "1.0.0").
PushChartToOCIRegistry("testdata/helm-values", "helm-values", "1.0.0").
HelmOCIRepoAdded("myrepo").
RepoURLType(fixture.RepoURLTypeHelmOCI).
Chart("helm-values").
@ -570,7 +570,7 @@ func TestHelmOCIRegistry(t *testing.T) {
func TestGitWithHelmOCIRegistryDependencies(t *testing.T) {
Given(t).
PushChartToOCIRegistry("helm-values", "helm-values", "1.0.0").
PushChartToOCIRegistry("testdata/helm-values", "helm-values", "1.0.0").
HelmOCIRepoAdded("myrepo").
Path("helm-oci-with-dependencies").
When().
@ -586,8 +586,8 @@ func TestGitWithHelmOCIRegistryDependencies(t *testing.T) {
func TestHelmOCIRegistryWithDependencies(t *testing.T) {
Given(t).
PushChartToOCIRegistry("helm-values", "helm-values", "1.0.0").
PushChartToOCIRegistry("helm-oci-with-dependencies", "helm-oci-with-dependencies", "1.0.0").
PushChartToOCIRegistry("testdata/helm-values", "helm-values", "1.0.0").
PushChartToOCIRegistry("testdata/helm-oci-with-dependencies", "helm-oci-with-dependencies", "1.0.0").
HelmOCIRepoAdded("myrepo").
RepoURLType(fixture.RepoURLTypeHelmOCI).
Chart("helm-oci-with-dependencies").
@ -605,7 +605,7 @@ func TestHelmOCIRegistryWithDependencies(t *testing.T) {
func TestTemplatesGitWithHelmOCIDependencies(t *testing.T) {
Given(t).
PushChartToOCIRegistry("helm-values", "helm-values", "1.0.0").
PushChartToOCIRegistry("testdata/helm-values", "helm-values", "1.0.0").
HelmoOCICredentialsWithoutUserPassAdded().
Path("helm-oci-with-dependencies").
When().
@ -621,8 +621,8 @@ func TestTemplatesGitWithHelmOCIDependencies(t *testing.T) {
func TestTemplatesHelmOCIWithDependencies(t *testing.T) {
Given(t).
PushChartToOCIRegistry("helm-values", "helm-values", "1.0.0").
PushChartToOCIRegistry("helm-oci-with-dependencies", "helm-oci-with-dependencies", "1.0.0").
PushChartToOCIRegistry("testdata/helm-values", "helm-values", "1.0.0").
PushChartToOCIRegistry("testdata/helm-oci-with-dependencies", "helm-oci-with-dependencies", "1.0.0").
HelmoOCICredentialsWithoutUserPassAdded().
RepoURLType(fixture.RepoURLTypeHelmOCI).
Chart("helm-oci-with-dependencies").

26
test/e2e/oci_test.go
View file

@ -14,7 +14,7 @@ import (
func TestOCIImage(t *testing.T) {
Given(t).
RepoURLType(fixture.RepoURLTypeOCI).
PushImageToOCIRegistry("guestbook", "1.0.0").
PushImageToOCIRegistry("testdata/guestbook", "1.0.0").
OCIRepoAdded("guestbook", "guestbook").
Revision("1.0.0").
OCIRegistry(fixture.OCIHostURL).
@ -37,8 +37,8 @@ func TestOCIImage(t *testing.T) {
func TestOCIWithOCIHelmRegistryDependencies(t *testing.T) {
Given(t).
RepoURLType(fixture.RepoURLTypeOCI).
PushChartToOCIRegistry("helm-values", "helm-values", "1.0.0").
PushImageToOCIRegistry("helm-oci-with-dependencies", "1.0.0").
PushChartToOCIRegistry("testdata/helm-values", "helm-values", "1.0.0").
PushImageToOCIRegistry("testdata/helm-oci-with-dependencies", "1.0.0").
OCIRegistry(fixture.OCIHostURL).
OCIRepoAdded("helm-oci-with-dependencies", "helm-oci-with-dependencies").
OCIRegistryPath("helm-oci-with-dependencies").
@ -58,8 +58,8 @@ func TestOCIWithOCIHelmRegistryDependencies(t *testing.T) {
func TestOCIWithAuthedOCIHelmRegistryDeps(t *testing.T) {
Given(t).
RepoURLType(fixture.RepoURLTypeOCI).
PushChartToAuthenticatedOCIRegistry("helm-values", "helm-values", "1.0.0").
PushImageToOCIRegistry("helm-oci-authed-with-dependencies", "1.0.0").
PushChartToAuthenticatedOCIRegistry("testdata/helm-values", "helm-values", "1.0.0").
PushImageToOCIRegistry("testdata/helm-oci-authed-with-dependencies", "1.0.0").
OCIRepoAdded("helm-oci-authed-with-dependencies", "helm-oci-authed-with-dependencies").
AuthenticatedOCIRepoAdded("helm-values", "myrepo/helm-values").
OCIRegistry(fixture.OCIHostURL).
@ -76,3 +76,19 @@ func TestOCIWithAuthedOCIHelmRegistryDeps(t *testing.T) {
Expect(HealthIs(health.HealthStatusHealthy)).
Expect(SyncStatusIs(SyncStatusCodeSynced))
}
func TestOCIImageWithOutOfBoundsSymlink(t *testing.T) {
Given(t).
RepoURLType(fixture.RepoURLTypeOCI).
PushImageToOCIRegistry("testdata3/symlink-out-of-bounds", "1.0.0").
OCIRepoAdded("symlink-out-of-bounds", "symlink-out-of-bounds").
Revision("1.0.0").
OCIRegistry(fixture.OCIHostURL).
OCIRegistryPath("symlink-out-of-bounds").
Path(".").
When().
IgnoreErrors().
CreateApp().
Then().
Expect(Error("", "could not decompress layer: illegal filepath in symlink"))
}

1
test/e2e/testdata3/README.md Normal file
View file

@ -0,0 +1 @@
The out-of-bounds symlinks can negatively affect the other testcases, therefore it is separated in its own testdata directory.

1
test/e2e/testdata3/symlink-out-of-bounds/out-of-bounds Symbolic link
View file

@ -0,0 +1 @@
..

2
util/cmp/stream.go
View file

@ -49,7 +49,7 @@ func ReceiveRepoStream(ctx context.Context, receiver StreamReceiver, destDir str
if err != nil {
return nil, fmt.Errorf("error receiving tgz file: %w", err)
}
err = files.Untgz(destDir, tgzFile, math.MaxInt64, preserveFileMode)
err = files.Untgz(destDir, tgzFile, math.MaxInt64, preserveFileMode, false)
if err != nil {
return nil, fmt.Errorf("error decompressing tgz file: %w", err)
}

2
util/helm/client.go
View file

@ -135,7 +135,7 @@ func untarChart(tempDir string, cachedChartPath string, manifestMaxExtractedSize
if err != nil {
return fmt.Errorf("error opening cached chart path %s: %w", cachedChartPath, err)
}
return files.Untgz(tempDir, reader, manifestMaxExtractedSize, false)
return files.Untgz(tempDir, reader, manifestMaxExtractedSize, false, false)
}
func (c *nativeHelmChart) ExtractChart(chart string, version string, passCredentials bool, manifestMaxExtractedSize int64, disableManifestMaxExtractedSize bool) (string, utilio.Closer, error) {

29
util/io/files/tar.go
View file

@ -71,7 +71,7 @@ func writeFile(srcPath string, inclusions []string, exclusions []string, writer
// - a full path
// - points to an empty directory or
// - points to a non-existing directory
func Untgz(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool) error {
func Untgz(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool, relativizeSymlinks bool) error {
if !filepath.IsAbs(dstPath) {
return fmt.Errorf("dstPath points to a relative path: %s", dstPath)
}
@ -81,7 +81,7 @@ func Untgz(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool) er
return fmt.Errorf("error reading file: %w", err)
}
defer gzr.Close()
return untar(dstPath, io.LimitReader(gzr, maxSize), preserveFileMode)
return untar(dstPath, io.LimitReader(gzr, maxSize), preserveFileMode, relativizeSymlinks)
}
// Untar will loop over the tar reader creating the file structure at dstPath.
@ -89,12 +89,12 @@ func Untgz(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool) er
// - a full path
// - points to an empty directory or
// - points to a non-existing directory
func Untar(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool) error {
func Untar(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool, relativizeSymlinks bool) error {
if !filepath.IsAbs(dstPath) {
return fmt.Errorf("dstPath points to a relative path: %s", dstPath)
}
return untar(dstPath, io.LimitReader(r, maxSize), preserveFileMode)
return untar(dstPath, io.LimitReader(r, maxSize), preserveFileMode, relativizeSymlinks)
}
// untar will loop over the tar reader creating the file structure at dstPath.
@ -102,7 +102,7 @@ func Untar(dstPath string, r io.Reader, maxSize int64, preserveFileMode bool) er
// - a full path
// - points to an empty directory or
// - points to a non existing directory
func untar(dstPath string, r io.Reader, preserveFileMode bool) error {
func untar(dstPath string, r io.Reader, preserveFileMode bool, relativizeSymlinks bool) error {
tr := tar.NewReader(r)
for {
@ -136,16 +136,27 @@ func untar(dstPath string, r io.Reader, preserveFileMode bool) error {
case tar.TypeSymlink:
// Sanity check to protect against symlink exploit
linkTarget := filepath.Join(filepath.Dir(target), header.Linkname)
realPath, err := filepath.EvalSymlinks(linkTarget)
realLinkTarget, err := filepath.EvalSymlinks(linkTarget)
if os.IsNotExist(err) {
realPath = linkTarget
realLinkTarget = linkTarget
} else if err != nil {
return fmt.Errorf("error checking symlink realpath: %w", err)
}
if !Inbound(realPath, dstPath) {
if !Inbound(realLinkTarget, dstPath) {
return fmt.Errorf("illegal filepath in symlink: %s", linkTarget)
}
err = os.Symlink(realPath, target)
if relativizeSymlinks {
// Relativizing all symlink targets because path.CheckOutOfBoundsSymlinks disallows any absolute symlinks
// and it makes more sense semantically to view symlinks in archives as relative.
// Inbound ensures that we never allow symlinks that break out of the target directory.
realLinkTarget, err = filepath.Rel(filepath.Dir(target), realLinkTarget)
if err != nil {
return fmt.Errorf("error relativizing link target: %w", err)
}
}
err = os.Symlink(realLinkTarget, target)
if err != nil {
return fmt.Errorf("error creating symlink: %w", err)
}

41
util/io/files/tar_test.go
View file

@ -159,7 +159,7 @@ func TestUntgz(t *testing.T) {
destDir := filepath.Join(tmpDir, "untgz1")
// when
err := files.Untgz(destDir, tgzFile, math.MaxInt64, false)
err := files.Untgz(destDir, tgzFile, math.MaxInt64, false, false)
// then
require.NoError(t, err)
@ -182,7 +182,23 @@ func TestUntgz(t *testing.T) {
destDir := filepath.Join(tmpDir, "untgz2")
// when
err := files.Untgz(destDir, tgzFile, math.MaxInt64, false)
err := files.Untgz(destDir, tgzFile, math.MaxInt64, false, false)
// then
assert.ErrorContains(t, err, "illegal filepath in symlink")
})
t.Run("will protect against symlink exploit when relativizing symlinks", func(t *testing.T) {
// given
tmpDir := createTmpDir(t)
defer deleteTmpDir(t, tmpDir)
tgzFile := createTgz(t, filepath.Join(getTestDataDir(t), "symlink-exploit"), tmpDir)
defer tgzFile.Close()
destDir := filepath.Join(tmpDir, "untgz2")
// when
err := files.Untgz(destDir, tgzFile, math.MaxInt64, false, true)
// then
assert.ErrorContains(t, err, "illegal filepath in symlink")
@ -198,14 +214,31 @@ func TestUntgz(t *testing.T) {
destDir := filepath.Join(tmpDir, "untgz1")
// when
err := files.Untgz(destDir, tgzFile, math.MaxInt64, false)
err := files.Untgz(destDir, tgzFile, math.MaxInt64, true, false)
require.NoError(t, err)
// then
scriptFileInfo, err := os.Stat(path.Join(destDir, "script.sh"))
require.NoError(t, err)
assert.Equal(t, os.FileMode(0o644), scriptFileInfo.Mode())
assert.Equal(t, os.FileMode(0o755), scriptFileInfo.Mode())
})
t.Run("relativizes symlinks", func(t *testing.T) {
// given
tmpDir := createTmpDir(t)
defer deleteTmpDir(t, tmpDir)
tgzFile := createTgz(t, getTestAppDir(t), tmpDir)
defer tgzFile.Close()
destDir := filepath.Join(tmpDir, "symlink-relativize")
// when
err := files.Untgz(destDir, tgzFile, math.MaxInt64, false, true)
// then
require.NoError(t, err)
names := readFiles(t, destDir)
assert.Equal(t, "../README.md", names["applicationset/readme-symlink"])
})
}

2
util/manifeststream/stream.go
View file

@ -183,7 +183,7 @@ func ReceiveManifestFileStream(ctx context.Context, receiver RepoStreamReceiver,
if err != nil {
return nil, nil, fmt.Errorf("error receiving tgz file: %w", err)
}
err = files.Untgz(destDir, tgzFile, maxExtractedSize, false)
err = files.Untgz(destDir, tgzFile, maxExtractedSize, false, false)
if err != nil {
return nil, nil, fmt.Errorf("error decompressing tgz file: %w", err)
}

4
util/oci/client.go
View file

@ -508,9 +508,9 @@ func (s *compressedLayerExtracterStore) Push(ctx context.Context, desc imagev1.D
defer os.RemoveAll(srcDir)
if strings.HasSuffix(desc.MediaType, "tar+gzip") {
err = files.Untgz(srcDir, content, s.maxSize, false)
err = files.Untgz(srcDir, content, s.maxSize, false, true)
} else {
err = files.Untar(srcDir, content, s.maxSize, false)
err = files.Untar(srcDir, content, s.maxSize, false, true)
}
if err != nil {