diff --git a/client/allocdir/alloc_dir.go b/client/allocdir/alloc_dir.go index 524769d607e9..8c52c01bfbfc 100644 --- a/client/allocdir/alloc_dir.go +++ b/client/allocdir/alloc_dir.go @@ -110,7 +110,7 @@ type AllocDirFS interface { Stat(path string) (*AllocFileInfo, error) ReadAt(path string, offset int64) (io.ReadCloser, error) Snapshot(w io.Writer) error - BlockUntilExists(path string, t *tomb.Tomb) chan error + BlockUntilExists(path string, t *tomb.Tomb) (chan error, error) ChangeEvents(path string, curOffset int64, t *tomb.Tomb) (*watch.FileChanges, error) } @@ -459,6 +459,12 @@ func (d *AllocDir) LogDir() string { // List returns the list of files at a path relative to the alloc dir func (d *AllocDir) List(path string) ([]*AllocFileInfo, error) { + if escapes, err := structs.PathEscapesAllocDir(path); err != nil { + return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err) + } else if escapes { + return nil, fmt.Errorf("Path escapes the alloc directory") + } + p := filepath.Join(d.AllocDir, path) finfos, err := ioutil.ReadDir(p) if err != nil { @@ -479,6 +485,12 @@ func (d *AllocDir) List(path string) ([]*AllocFileInfo, error) { // Stat returns information about the file at a path relative to the alloc dir func (d *AllocDir) Stat(path string) (*AllocFileInfo, error) { + if escapes, err := structs.PathEscapesAllocDir(path); err != nil { + return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err) + } else if escapes { + return nil, fmt.Errorf("Path escapes the alloc directory") + } + p := filepath.Join(d.AllocDir, path) info, err := os.Stat(p) if err != nil { @@ -496,6 +508,12 @@ func (d *AllocDir) Stat(path string) (*AllocFileInfo, error) { // ReadAt returns a reader for a file at the path relative to the alloc dir func (d *AllocDir) ReadAt(path string, offset int64) (io.ReadCloser, error) { + if escapes, err := structs.PathEscapesAllocDir(path); err != nil { + return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err) + } else if escapes { + return nil, fmt.Errorf("Path escapes the alloc directory") + } + p := filepath.Join(d.AllocDir, path) f, err := os.Open(p) if err != nil { @@ -509,7 +527,13 @@ func (d *AllocDir) ReadAt(path string, offset int64) (io.ReadCloser, error) { // BlockUntilExists blocks until the passed file relative the allocation // directory exists. The block can be cancelled with the passed tomb. -func (d *AllocDir) BlockUntilExists(path string, t *tomb.Tomb) chan error { +func (d *AllocDir) BlockUntilExists(path string, t *tomb.Tomb) (chan error, error) { + if escapes, err := structs.PathEscapesAllocDir(path); err != nil { + return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err) + } else if escapes { + return nil, fmt.Errorf("Path escapes the alloc directory") + } + // Get the path relative to the alloc directory p := filepath.Join(d.AllocDir, path) watcher := getFileWatcher(p) @@ -518,13 +542,19 @@ func (d *AllocDir) BlockUntilExists(path string, t *tomb.Tomb) chan error { returnCh <- watcher.BlockUntilExists(t) close(returnCh) }() - return returnCh + return returnCh, nil } // ChangeEvents watches for changes to the passed path relative to the // allocation directory. The offset should be the last read offset. The tomb is // used to clean up the watch. func (d *AllocDir) ChangeEvents(path string, curOffset int64, t *tomb.Tomb) (*watch.FileChanges, error) { + if escapes, err := structs.PathEscapesAllocDir(path); err != nil { + return nil, fmt.Errorf("Failed to check if path escapes alloc directory: %v", err) + } else if escapes { + return nil, fmt.Errorf("Path escapes the alloc directory") + } + // Get the path relative to the alloc directory p := filepath.Join(d.AllocDir, path) watcher := getFileWatcher(p) diff --git a/client/allocdir/alloc_dir_test.go b/client/allocdir/alloc_dir_test.go index 712b01cf1c1a..86eb6f4c57e3 100644 --- a/client/allocdir/alloc_dir_test.go +++ b/client/allocdir/alloc_dir_test.go @@ -9,8 +9,11 @@ import ( "path/filepath" "reflect" "runtime" + "strings" "testing" + tomb "gopkg.in/tomb.v1" + "github.com/hashicorp/nomad/client/testutil" "github.com/hashicorp/nomad/nomad/structs" ) @@ -345,3 +348,45 @@ func TestAllocDir_Move(t *testing.T) { t.Fatalf("task local dir was not moved") } } + +func TestAllocDir_EscapeChecking(t *testing.T) { + tmp, err := ioutil.TempDir("", "AllocDir") + if err != nil { + t.Fatalf("Couldn't create temp dir: %v", err) + } + defer os.RemoveAll(tmp) + + d := NewAllocDir(tmp, structs.DefaultResources().DiskMB) + defer d.Destroy() + tasks := []*structs.Task{t1, t2} + if err := d.Build(tasks); err != nil { + t.Fatalf("Build(%v) failed: %v", tasks, err) + } + + // Check that issuing calls that escape the alloc dir returns errors + // List + if _, err := d.List(".."); err == nil || !strings.Contains(err.Error(), "escapes") { + t.Fatalf("List of escaping path didn't error: %v", err) + } + + // Stat + if _, err := d.Stat("../foo"); err == nil || !strings.Contains(err.Error(), "escapes") { + t.Fatalf("Stat of escaping path didn't error: %v", err) + } + + // ReadAt + if _, err := d.ReadAt("../foo", 0); err == nil || !strings.Contains(err.Error(), "escapes") { + t.Fatalf("ReadAt of escaping path didn't error: %v", err) + } + + // BlockUntilExists + tomb := tomb.Tomb{} + if _, err := d.BlockUntilExists("../foo", &tomb); err == nil || !strings.Contains(err.Error(), "escapes") { + t.Fatalf("BlockUntilExists of escaping path didn't error: %v", err) + } + + // ChangeEvents + if _, err := d.ChangeEvents("../foo", 0, &tomb); err == nil || !strings.Contains(err.Error(), "escapes") { + t.Fatalf("ChangeEvents of escaping path didn't error: %v", err) + } +} diff --git a/command/agent/fs_endpoint.go b/command/agent/fs_endpoint.go index 342098429a62..a70546e604b4 100644 --- a/command/agent/fs_endpoint.go +++ b/command/agent/fs_endpoint.go @@ -856,7 +856,13 @@ func blockUntilNextLog(fs allocdir.AllocDirFS, t *tomb.Tomb, logPath, task, logT next := make(chan error, 1) go func() { - eofCancelCh := fs.BlockUntilExists(nextPath, t) + eofCancelCh, err := fs.BlockUntilExists(nextPath, t) + if err != nil { + next <- err + close(next) + return + } + scanCh := time.Tick(nextLogCheckRate) for { select { diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go index fa40f84a94ec..aefa53c23a7d 100644 --- a/nomad/structs/structs.go +++ b/nomad/structs/structs.go @@ -2274,7 +2274,7 @@ func (t *Template) Validate() error { } // Verify the destination doesn't escape - escaped, err := pathEscapesAllocDir(t.DestPath) + escaped, err := PathEscapesAllocDir(t.DestPath) if err != nil { mErr.Errors = append(mErr.Errors, fmt.Errorf("invalid destination path: %v", err)) } else if escaped { @@ -2593,9 +2593,9 @@ func (ta *TaskArtifact) GoString() string { return fmt.Sprintf("%+v", ta) } -// pathEscapesAllocDir returns if the given path escapes the allocation +// PathEscapesAllocDir returns if the given path escapes the allocation // directory -func pathEscapesAllocDir(path string) (bool, error) { +func PathEscapesAllocDir(path string) (bool, error) { // Verify the destination doesn't escape the tasks directory alloc, err := filepath.Abs(filepath.Join("/", "foo/", "bar/")) if err != nil { @@ -2620,7 +2620,7 @@ func (ta *TaskArtifact) Validate() error { mErr.Errors = append(mErr.Errors, fmt.Errorf("source must be specified")) } - escaped, err := pathEscapesAllocDir(ta.RelativeDest) + escaped, err := PathEscapesAllocDir(ta.RelativeDest) if err != nil { mErr.Errors = append(mErr.Errors, fmt.Errorf("invalid destination path: %v", err)) } else if escaped {