Skip to content

Commit e24dcd7

Browse files
fix: prevent path traversal in saveParameter and saveArtifact
Add bounds check before writing output parameter/artifact files so that relative paths like ../../ctr/containerB/exitcode cannot escape the /var/run/argo/outputs/ subtree. Fixes GHSA-r9w2-rp4m-gwr9 Signed-off-by: Jayant Kamble <jayantkamble10000@google.com> Signed-off-by: Jayant Kamble <jayantkamble10000@gmail.com>
1 parent 6f09479 commit e24dcd7

2 files changed

Lines changed: 57 additions & 2 deletions

File tree

cmd/argoexec/commands/emissary.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -401,11 +401,15 @@ func saveArtifact(ctx context.Context, srcPath string) error {
401401
logger.WithField("srcPath", srcPath).Info(ctx, "no need to save artifact - on overlapping volume")
402402
return nil
403403
}
404+
dstPath := filepath.Join(varRunArgo, "outputs/artifacts", strings.TrimSuffix(srcPath, "/")+".tgz")
405+
safeArtifactBase := filepath.Clean(filepath.Join(varRunArgo, "outputs/artifacts")) + string(os.PathSeparator)
406+
if !strings.HasPrefix(filepath.Clean(dstPath)+string(os.PathSeparator), safeArtifactBase) {
407+
return fmt.Errorf("path traversal in output artifact path %q", srcPath)
408+
}
404409
if _, err := os.Stat(srcPath); os.IsNotExist(err) { // might be optional, so we ignore
405410
logger.WithField("srcPath", srcPath).WithError(err).Warn(ctx, "cannot save artifact")
406411
return nil
407412
}
408-
dstPath := filepath.Join(varRunArgo, "/outputs/artifacts/", strings.TrimSuffix(srcPath, "/")+".tgz")
409413
logger.WithFields(logging.Fields{
410414
"src": srcPath,
411415
"dst": dstPath,
@@ -435,6 +439,11 @@ func saveParameter(ctx context.Context, srcPath string) error {
435439
logger.WithField("src", srcPath).Info(ctx, "no need to save parameter - on overlapping volume")
436440
return nil
437441
}
442+
dstPath := filepath.Join(varRunArgo, "outputs/parameters", srcPath)
443+
safeParamBase := filepath.Clean(filepath.Join(varRunArgo, "outputs/parameters")) + string(os.PathSeparator)
444+
if !strings.HasPrefix(filepath.Clean(dstPath)+string(os.PathSeparator), safeParamBase) {
445+
return fmt.Errorf("path traversal in output parameter path %q", srcPath)
446+
}
438447
src, err := os.Open(filepath.Clean(srcPath))
439448
if os.IsNotExist(err) { // might be optional, so we ignore
440449
logger.WithField("src", srcPath).WithError(err).Error(ctx, "cannot save parameter, does not exist")
@@ -444,7 +453,6 @@ func saveParameter(ctx context.Context, srcPath string) error {
444453
return fmt.Errorf("failed to open %s: %w", srcPath, err)
445454
}
446455
defer func() { _ = src.Close() }()
447-
dstPath := varRunArgo + "/outputs/parameters/" + srcPath
448456
logger.WithFields(logging.Fields{
449457
"src": srcPath,
450458
"dst": dstPath,

cmd/argoexec/commands/emissary_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package commands
55
import (
66
"fmt"
77
"os"
8+
"path/filepath"
89
"strconv"
910
"sync"
1011
"syscall"
@@ -205,6 +206,52 @@ func TestEmissary(t *testing.T) {
205206
})
206207
}
207208

209+
func TestSaveParameterPathTraversal(t *testing.T) {
210+
tmp := t.TempDir()
211+
varRunArgo = tmp
212+
ctx := logging.TestContext(t.Context())
213+
214+
srcFile := filepath.Join(tmp, "result.txt")
215+
require.NoError(t, os.WriteFile(srcFile, []byte("hello"), 0o644))
216+
217+
t.Run("LegitimateAbsolutePath", func(t *testing.T) {
218+
err := saveParameter(ctx, srcFile)
219+
require.NoError(t, err)
220+
})
221+
t.Run("TraversalToArgoexec", func(t *testing.T) {
222+
err := saveParameter(ctx, "../../argoexec")
223+
require.Error(t, err)
224+
assert.Contains(t, err.Error(), "path traversal")
225+
})
226+
t.Run("TraversalToSidecarExitCode", func(t *testing.T) {
227+
err := saveParameter(ctx, "../../ctr/sidecar/exitcode")
228+
require.Error(t, err)
229+
assert.Contains(t, err.Error(), "path traversal")
230+
})
231+
t.Run("TraversalToTemplate", func(t *testing.T) {
232+
err := saveParameter(ctx, "../template")
233+
require.Error(t, err)
234+
assert.Contains(t, err.Error(), "path traversal")
235+
})
236+
}
237+
238+
func TestSaveArtifactPathTraversal(t *testing.T) {
239+
tmp := t.TempDir()
240+
varRunArgo = tmp
241+
ctx := logging.TestContext(t.Context())
242+
243+
t.Run("TraversalToArgoexec", func(t *testing.T) {
244+
err := saveArtifact(ctx, "../../argoexec")
245+
require.Error(t, err)
246+
assert.Contains(t, err.Error(), "path traversal")
247+
})
248+
t.Run("TraversalToSidecarExitCode", func(t *testing.T) {
249+
err := saveArtifact(ctx, "../../ctr/sidecar/exitcode")
250+
require.Error(t, err)
251+
assert.Contains(t, err.Error(), "path traversal")
252+
})
253+
}
254+
208255
func run(script string) error {
209256
cmd := NewEmissaryCommand()
210257
_, _, err := cmdutil.ContextWithLogger(cmd, string(logging.Info), string(logging.Text))

0 commit comments

Comments
 (0)