cleanup: move fs helpers into escapingfs

This commit is contained in:
Seth Hoenig
2022-08-24 09:56:42 -05:00
parent 7f5dfe4478
commit 1b1a68e42f
9 changed files with 119 additions and 124 deletions

View File

@@ -19,7 +19,7 @@ import (
clientconfig "github.com/hashicorp/nomad/client/config"
"github.com/hashicorp/nomad/client/interfaces"
"github.com/hashicorp/nomad/client/taskenv"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/escapingfs"
"github.com/hashicorp/nomad/nomad/mock"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/stretchr/testify/require"
@@ -31,11 +31,11 @@ type noopReplacer struct {
}
func clientPath(taskDir, path string, join bool) (string, bool) {
if !filepath.IsAbs(path) || (helper.PathEscapesSandbox(taskDir, path) && join) {
if !filepath.IsAbs(path) || (escapingfs.PathEscapesSandbox(taskDir, path) && join) {
path = filepath.Join(taskDir, path)
}
path = filepath.Clean(path)
if taskDir != "" && !helper.PathEscapesSandbox(taskDir, path) {
if taskDir != "" && !escapingfs.PathEscapesSandbox(taskDir, path) {
return path, false
}
return path, true

View File

@@ -11,6 +11,7 @@ import (
"github.com/hashicorp/nomad/helper"
hargs "github.com/hashicorp/nomad/helper/args"
"github.com/hashicorp/nomad/helper/escapingfs"
"github.com/hashicorp/nomad/lib/cpuset"
"github.com/hashicorp/nomad/nomad/structs"
"github.com/hashicorp/nomad/plugins/drivers"
@@ -341,7 +342,7 @@ func (t *TaskEnv) replaceEnvClient(arg string) string {
// directory path fields of this TaskEnv
func (t *TaskEnv) checkEscape(testPath string) bool {
for _, p := range []string{t.clientTaskDir, t.clientSharedAllocDir} {
if p != "" && !helper.PathEscapesSandbox(p, testPath) {
if p != "" && !escapingfs.PathEscapesSandbox(p, testPath) {
return false
}
}

View File

@@ -24,8 +24,8 @@ import (
"github.com/hashicorp/nomad/client/state"
"github.com/hashicorp/nomad/command/agent/consul"
"github.com/hashicorp/nomad/command/agent/event"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/bufconndialer"
"github.com/hashicorp/nomad/helper/escapingfs"
"github.com/hashicorp/nomad/helper/pluginutils/loader"
"github.com/hashicorp/nomad/helper/uuid"
"github.com/hashicorp/nomad/lib/cpuset"
@@ -868,7 +868,7 @@ func (a *Agent) setupNodeID(config *nomad.Config) error {
return err
}
// Persist this configured nodeID to our data directory
if err := helper.EnsurePath(fileID, false); err != nil {
if err := escapingfs.EnsurePath(fileID, false); err != nil {
return err
}
if err := ioutil.WriteFile(fileID, []byte(config.NodeID), 0600); err != nil {
@@ -880,7 +880,7 @@ func (a *Agent) setupNodeID(config *nomad.Config) error {
// If we still don't have a valid node ID, make one.
if config.NodeID == "" {
id := uuid.Generate()
if err := helper.EnsurePath(fileID, false); err != nil {
if err := escapingfs.EnsurePath(fileID, false); err != nil {
return err
}
if err := ioutil.WriteFile(fileID, []byte(id), 0600); err != nil {

View File

@@ -27,6 +27,7 @@ import (
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/api/contexts"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/escapingfs"
"github.com/hashicorp/nomad/version"
"github.com/posener/complete"
)
@@ -729,7 +730,7 @@ func (c *OperatorDebugCommand) mkdir(paths ...string) error {
joinedPath := c.path(paths...)
// Ensure path doesn't escape the sandbox of the capture directory
escapes := helper.PathEscapesSandbox(c.collectDir, joinedPath)
escapes := escapingfs.PathEscapesSandbox(c.collectDir, joinedPath)
if escapes {
return fmt.Errorf("file path escapes capture directory")
}
@@ -1273,7 +1274,7 @@ func (c *OperatorDebugCommand) writeBytes(dir, file string, data []byte) error {
}
// Ensure filename doesn't escape the sandbox of the capture directory
escapes := helper.PathEscapesSandbox(c.collectDir, filePath)
escapes := escapingfs.PathEscapesSandbox(c.collectDir, filePath)
if escapes {
return fmt.Errorf("file path \"%s\" escapes capture directory \"%s\"", filePath, c.collectDir)
}

View File

@@ -97,3 +97,25 @@ func PathEscapesAllocDir(base, prefix, path string) (bool, error) {
return false, nil
}
// PathEscapesSandbox returns whether previously cleaned path inside the
// sandbox directory (typically this will be the allocation directory)
// escapes.
func PathEscapesSandbox(sandboxDir, path string) bool {
rel, err := filepath.Rel(sandboxDir, path)
if err != nil {
return true
}
if strings.HasPrefix(rel, "..") {
return true
}
return false
}
// EnsurePath is used to make sure a path exists
func EnsurePath(path string, dir bool) error {
if !dir {
path = filepath.Dir(path)
}
return os.MkdirAll(path, 0755)
}

View File

@@ -1,6 +1,7 @@
package escapingfs
import (
"fmt"
"io/ioutil"
"os"
"path/filepath"
@@ -141,3 +142,88 @@ func Test_PathEscapesAllocDir(t *testing.T) {
require.True(t, escape)
})
}
func TestPathEscapesSandbox(t *testing.T) {
cases := []struct {
name string
path string
dir string
expected bool
}{
{
// this is the ${NOMAD_SECRETS_DIR} case
name: "ok joined absolute path inside sandbox",
path: filepath.Join("/alloc", "/secrets"),
dir: "/alloc",
expected: false,
},
{
name: "fail unjoined absolute path outside sandbox",
path: "/secrets",
dir: "/alloc",
expected: true,
},
{
name: "ok joined relative path inside sandbox",
path: filepath.Join("/alloc", "./safe"),
dir: "/alloc",
expected: false,
},
{
name: "fail unjoined relative path outside sandbox",
path: "./safe",
dir: "/alloc",
expected: true,
},
{
name: "ok relative path traversal constrained to sandbox",
path: filepath.Join("/alloc", "../../alloc/safe"),
dir: "/alloc",
expected: false,
},
{
name: "ok unjoined absolute path traversal constrained to sandbox",
path: filepath.Join("/alloc", "/../alloc/safe"),
dir: "/alloc",
expected: false,
},
{
name: "ok unjoined absolute path traversal constrained to sandbox",
path: "/../alloc/safe",
dir: "/alloc",
expected: false,
},
{
name: "fail joined relative path traverses outside sandbox",
path: filepath.Join("/alloc", "../../../unsafe"),
dir: "/alloc",
expected: true,
},
{
name: "fail unjoined relative path traverses outside sandbox",
path: "../../../unsafe",
dir: "/alloc",
expected: true,
},
{
name: "fail joined absolute path tries to transverse outside sandbox",
path: filepath.Join("/alloc", "/alloc/../../unsafe"),
dir: "/alloc",
expected: true,
},
{
name: "fail unjoined absolute path tries to transverse outside sandbox",
path: "/alloc/../../unsafe",
dir: "/alloc",
expected: true,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
caseMsg := fmt.Sprintf("path: %v\ndir: %v", tc.path, tc.dir)
escapes := PathEscapesSandbox(tc.dir, tc.path)
require.Equal(t, tc.expected, escapes, caseMsg)
})
}
}

View File

@@ -609,20 +609,6 @@ func CheckNamespaceScope(provided string, requested []string) []string {
return nil
}
// PathEscapesSandbox returns whether previously cleaned path inside the
// sandbox directory (typically this will be the allocation directory)
// escapes.
func PathEscapesSandbox(sandboxDir, path string) bool {
rel, err := filepath.Rel(sandboxDir, path)
if err != nil {
return true
}
if strings.HasPrefix(rel, "..") {
return true
}
return false
}
// StopFunc is used to stop a time.Timer created with NewSafeTimer
type StopFunc func()

View File

@@ -2,7 +2,6 @@ package helper
import (
"fmt"
"path/filepath"
"reflect"
"sort"
"testing"
@@ -449,91 +448,6 @@ func TestCheckNamespaceScope(t *testing.T) {
}
}
func TestPathEscapesSandbox(t *testing.T) {
cases := []struct {
name string
path string
dir string
expected bool
}{
{
// this is the ${NOMAD_SECRETS_DIR} case
name: "ok joined absolute path inside sandbox",
path: filepath.Join("/alloc", "/secrets"),
dir: "/alloc",
expected: false,
},
{
name: "fail unjoined absolute path outside sandbox",
path: "/secrets",
dir: "/alloc",
expected: true,
},
{
name: "ok joined relative path inside sandbox",
path: filepath.Join("/alloc", "./safe"),
dir: "/alloc",
expected: false,
},
{
name: "fail unjoined relative path outside sandbox",
path: "./safe",
dir: "/alloc",
expected: true,
},
{
name: "ok relative path traversal constrained to sandbox",
path: filepath.Join("/alloc", "../../alloc/safe"),
dir: "/alloc",
expected: false,
},
{
name: "ok unjoined absolute path traversal constrained to sandbox",
path: filepath.Join("/alloc", "/../alloc/safe"),
dir: "/alloc",
expected: false,
},
{
name: "ok unjoined absolute path traversal constrained to sandbox",
path: "/../alloc/safe",
dir: "/alloc",
expected: false,
},
{
name: "fail joined relative path traverses outside sandbox",
path: filepath.Join("/alloc", "../../../unsafe"),
dir: "/alloc",
expected: true,
},
{
name: "fail unjoined relative path traverses outside sandbox",
path: "../../../unsafe",
dir: "/alloc",
expected: true,
},
{
name: "fail joined absolute path tries to transverse outside sandbox",
path: filepath.Join("/alloc", "/alloc/../../unsafe"),
dir: "/alloc",
expected: true,
},
{
name: "fail unjoined absolute path tries to transverse outside sandbox",
path: "/alloc/../../unsafe",
dir: "/alloc",
expected: true,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
caseMsg := fmt.Sprintf("path: %v\ndir: %v", tc.path, tc.dir)
escapes := PathEscapesSandbox(tc.dir, tc.path)
require.Equal(t, tc.expected, escapes, caseMsg)
})
}
}
func Test_NewSafeTimer(t *testing.T) {
t.Run("zero", func(t *testing.T) {
timer, stop := NewSafeTimer(0)

View File

@@ -1,15 +0,0 @@
// These functions are coming from consul/path.go
package helper
import (
"os"
"path/filepath"
)
// EnsurePath is used to make sure a path exists
func EnsurePath(path string, dir bool) error {
if !dir {
path = filepath.Dir(path)
}
return os.MkdirAll(path, 0755)
}