fix: prevent path traversal in saveParameter and saveArtifact

Normalize absolute source paths by stripping the leading slash before
building the destination path (e.g. /tmp/artifact → tmp/artifact).
Validate the resulting relative path with filepath.IsLocal to block
any .. traversal, then create the destination file through os.OpenRoot
so the OS enforces the directory boundary and symlink escapes are also
prevented.

Signed-off-by: Jayant Kamble <jayantkamble10000@gmail.com>

Author: jayantkamble10000

cmd/argoexec/commands/emissary.go

@@ -401,20 +401,33 @@ func saveArtifact(ctx context.Context, srcPath string) error {
 		logger.WithField("srcPath", srcPath).Info(ctx, "no need to save artifact - on overlapping volume")
 		return nil
 	}
+	// Normalize absolute paths (e.g. /tmp/artifact → tmp/artifact) so that
+	// filepath.IsLocal can validate the result, and os.Root.Create can safely
+	// write it within the outputs directory.
+	relSrc := strings.TrimPrefix(strings.TrimSuffix(srcPath, "/"), "/")
+	dstRel := relSrc + ".tgz"
+	if !filepath.IsLocal(dstRel) {
+		return fmt.Errorf("path traversal in output artifact path %q", srcPath)
+	}
+	artifactsDir := filepath.Join(varRunArgo, "outputs/artifacts")
+	dstPath := filepath.Join(artifactsDir, dstRel)
 	if _, err := os.Stat(srcPath); os.IsNotExist(err) { // might be optional, so we ignore
 		logger.WithField("srcPath", srcPath).WithError(err).Warn(ctx, "cannot save artifact")
 		return nil
 	}
-	dstPath := filepath.Join(varRunArgo, "/outputs/artifacts/", strings.TrimSuffix(srcPath, "/")+".tgz")
 	logger.WithFields(logging.Fields{
 		"src": srcPath,
 		"dst": dstPath,
 	}).Info(ctx, "saving artifact")
-	z := filepath.Dir(dstPath)
-	if err := os.MkdirAll(z, 0o755); err != nil { // chmod rwxr-xr-x
-		return fmt.Errorf("failed to create directory %s: %w", z, err)
+	if err := os.MkdirAll(filepath.Dir(dstPath), 0o755); err != nil { // chmod rwxr-xr-x
+		return fmt.Errorf("failed to create directory %s: %w", filepath.Dir(dstPath), err)
+	}
+	root, err := os.OpenRoot(artifactsDir)
+	if err != nil {
+		return fmt.Errorf("failed to open artifacts output root: %w", err)
 	}
-	dst, err := os.Create(dstPath)
+	defer root.Close()
+	dst, err := root.Create(dstRel)
 	if err != nil {
 		return fmt.Errorf("failed to create destination %s: %w", dstPath, err)
 	}
@@ -435,6 +448,15 @@ func saveParameter(ctx context.Context, srcPath string) error {
 		logger.WithField("src", srcPath).Info(ctx, "no need to save parameter - on overlapping volume")
 		return nil
 	}
+	// Normalize absolute paths (e.g. /tmp/parameter → tmp/parameter) so that
+	// filepath.IsLocal can validate the result, and os.Root.Create can safely
+	// write it within the outputs directory.
+	relPath := strings.TrimPrefix(srcPath, "/")
+	if !filepath.IsLocal(relPath) {
+		return fmt.Errorf("path traversal in output parameter path %q", srcPath)
+	}
+	parametersDir := filepath.Join(varRunArgo, "outputs/parameters")
+	dstPath := filepath.Join(parametersDir, relPath)
 	src, err := os.Open(filepath.Clean(srcPath))
 	if os.IsNotExist(err) { // might be optional, so we ignore
 		logger.WithField("src", srcPath).WithError(err).Error(ctx, "cannot save parameter, does not exist")
@@ -444,16 +466,19 @@ func saveParameter(ctx context.Context, srcPath string) error {
 		return fmt.Errorf("failed to open %s: %w", srcPath, err)
 	}
 	defer func() { _ = src.Close() }()
-	dstPath := varRunArgo + "/outputs/parameters/" + srcPath
 	logger.WithFields(logging.Fields{
 		"src": srcPath,
 		"dst": dstPath,
 	}).Info(ctx, "saving parameter")
-	z := filepath.Dir(dstPath)
-	if mkdirErr := os.MkdirAll(z, 0o755); mkdirErr != nil { // chmod rwxr-xr-x
-		return fmt.Errorf("failed to create directory %s: %w", z, mkdirErr)
+	if mkdirErr := os.MkdirAll(filepath.Dir(dstPath), 0o755); mkdirErr != nil { // chmod rwxr-xr-x
+		return fmt.Errorf("failed to create directory %s: %w", filepath.Dir(dstPath), mkdirErr)
+	}
+	root, err := os.OpenRoot(parametersDir)
+	if err != nil {
+		return fmt.Errorf("failed to open parameters output root: %w", err)
 	}
-	dst, err := os.Create(dstPath)
+	defer root.Close()
+	dst, err := root.Create(relPath)
 	if err != nil {
 		return fmt.Errorf("failed to create %s: %w", srcPath, err)
 	}

cmd/argoexec/commands/emissary_test.go

@@ -5,6 +5,7 @@ package commands
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"strconv"
 	"sync"
 	"syscall"
@@ -205,6 +206,52 @@ func TestEmissary(t *testing.T) {
 	})
 }
 
+func TestSaveParameterPathTraversal(t *testing.T) {
+	tmp := t.TempDir()
+	varRunArgo = tmp
+	ctx := logging.TestContext(t.Context())
+
+	srcFile := "result.txt"
+	require.NoError(t, os.WriteFile(filepath.Join(tmp, srcFile), []byte("hello"), 0o644))
+
+	t.Run("LegitimateRelativePath", func(t *testing.T) {
+		err := saveParameter(ctx, srcFile)
+		require.NoError(t, err)
+	})
+	t.Run("TraversalToArgoexec", func(t *testing.T) {
+		err := saveParameter(ctx, "../../argoexec")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "path traversal")
+	})
+	t.Run("TraversalToSidecarExitCode", func(t *testing.T) {
+		err := saveParameter(ctx, "../../ctr/sidecar/exitcode")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "path traversal")
+	})
+	t.Run("TraversalToTemplate", func(t *testing.T) {
+		err := saveParameter(ctx, "../template")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "path traversal")
+	})
+}
+
+func TestSaveArtifactPathTraversal(t *testing.T) {
+	tmp := t.TempDir()
+	varRunArgo = tmp
+	ctx := logging.TestContext(t.Context())
+
+	t.Run("TraversalToArgoexec", func(t *testing.T) {
+		err := saveArtifact(ctx, "../../argoexec")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "path traversal")
+	})
+	t.Run("TraversalToSidecarExitCode", func(t *testing.T) {
+		err := saveArtifact(ctx, "../../ctr/sidecar/exitcode")
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "path traversal")
+	})
+}
+
 func run(script string) error {
 	cmd := NewEmissaryCommand()
 	_, _, err := cmdutil.ContextWithLogger(cmd, string(logging.Info), string(logging.Text))