Skip to content

Commit

Permalink
file sandbox: check but don't alter paths passed to 'file'
Browse files Browse the repository at this point in the history
  • Loading branch information
tgross committed Aug 9, 2019
1 parent 0d999b3 commit 659167f
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 51 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,8 @@ template {
function_blacklist = []
# If a sandbox path is provided, any path provided to the `file` function is
# prefixed by the sandbox path. Relative paths that try to traverse outside
# that prefix will exit with an error.
# checked that it falls within the sandbox path. Relative paths that try to
# traverse outside the sandbox path will exit with an error.
sandbox_path = ""
# This is the `minimum(:maximum)` to wait before rendering a new template to
Expand Down
25 changes: 12 additions & 13 deletions template/funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ func fileFunc(b *Brain, used, missing *dep.Set, sandboxPath string) func(string)
if len(s) == 0 {
return "", nil
}
s, err := sandboxedPath(sandboxPath, s)
ok, err := pathInSandbox(sandboxPath, s)
if !ok {
return "", fmt.Errorf("'%s' is outside of sandbox", s)
}
if err != nil {
return "", err
}
Expand Down Expand Up @@ -1166,26 +1169,22 @@ func blacklisted(...string) (string, error) {
return "", errors.New("function is disabled")
}

// sandboxedPath returns a path that's been prefixed by the sandbox path,
// if any. If a sandbox path was provided, it will return an error if the
// path falls outside the sandbox.
func sandboxedPath(sandbox, s string) (string, error) {
path, err := filepath.Abs(filepath.Join(sandbox, s))
if err != nil {
return "", err
}
// pathInSandbox determines whether a provided path falls within the sandbox.
// returns an error only if the file can't be evaluated (missing, invalid
// symlink, etc.)
func pathInSandbox(sandbox, path string) (bool, error) {
if sandbox != "" {
path, err := filepath.EvalSymlinks(path)
if err != nil {
return "", err
return false, err
}
path, err = filepath.Rel(sandbox, path)
if err != nil {
return "", err
return false, err
}
if strings.HasPrefix(path, "..") {
return "", fmt.Errorf("'%s' is outside of sandbox", s)
return false, nil
}
}
return path, nil
return true, nil
}
55 changes: 19 additions & 36 deletions template/funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,91 +2,74 @@ package template

import (
"fmt"
"os"
"path/filepath"
"runtime"
"testing"

"github.com/pkg/errors"
)

func TestFileSandbox(t *testing.T) {
// while most of the function can be tested lexigraphically,
// we need to be able to walk actual symlinks.
_, filename, _, _ := runtime.Caller(0)
sandboxDir := filepath.Join(filepath.Dir(filename), "testdata", "sandbox")
cwd, _ := os.Getwd()
cases := []struct {
name string
sandbox string
path string
expectedPath string
expectedErr error
name string
sandbox string
path string
expected bool
}{
{
"absolute_path_no_sandbox",
"",
"/path/to/file",
"/path/to/file",
nil,
true,
},
{
"relative_path_no_sandbox",
"",
"./path/to/file",
filepath.Join(cwd, "path/to/file"),
nil,
true,
},
{
"absolute_path_with_sandbox",
sandboxDir,
"/path/to/file",
filepath.Join(sandboxDir, "path/to/file"),
nil,
true,
},
{
"relative_path_in_sandbox",
sandboxDir,
"./path/to/file",
filepath.Join(sandboxDir, "path/to/file"),
nil,
filepath.Join(sandboxDir, "path/to/../to/file"),
true,
},
{
"symlink_path_in_sandbox",
sandboxDir,
"./path/to/ok-symlink",
filepath.Join(sandboxDir, "path/to/ok-symlink"),
nil,
true,
},
{
"relative_path_escaping_sandbox",
sandboxDir,
"/path/../../../funcs_test.go",
"",
errors.New("'/path/../../../funcs_test.go' is outside of sandbox"),
filepath.Join(sandboxDir, "path/../../../funcs_test.go"),
false,
},
{
"symlink_escaping_sandbox",
sandboxDir,
"/path/to/bad-symlink",
"",
errors.New("'/path/to/bad-symlink' is outside of sandbox"),
filepath.Join(sandboxDir, "path/to/bad-symlink"),
false,
},
}

for i, tc := range cases {
t.Run(fmt.Sprintf("%d_%s", i, tc.name), func(t *testing.T) {
result, err := sandboxedPath(tc.sandbox, tc.path)
if tc.expectedErr != nil {
if err == nil {
t.Fatalf("expected error %s got nil", tc.expectedErr)
}
if err.Error() != tc.expectedErr.Error() {
t.Fatalf("expected %s got %s", tc.expectedErr, err)
}
result, err := pathInSandbox(tc.sandbox, tc.path)
if err != nil {
t.Fatal(err)
}
if result != tc.expectedPath {
t.Fatalf("expected %s got %s", tc.expectedPath, result)
if result != tc.expected {
t.Fatalf("expected %v got %v", tc.expected, result)
}
})
}
Expand Down

0 comments on commit 659167f

Please sign in to comment.