cmd/txtar: extract only in current dir by default
Change the behavior of txtar so that files are extracted only in the
current directory, or a subdirectory of the current directory, by
default.
If the archive contains a path outside the current directory, txtar will
print an error message and quit with exit code 1. It will not extract
any files in this case. This also applies if an environment variable
used in the archive expands into a path outside the current directory.
Add flag -unsafe to remove the restriction and allow extracting files
outside the current dir.
Updates golang/go#46741
Change-Id: Ic12fb8286c5f2a930addd82dcfce196d4a04054c
Reviewed-on: https://go-review.googlesource.com/c/exp/+/371274
Trust: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Trust: Bryan Mills <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/cmd/txtar/txtar.go b/cmd/txtar/txtar.go
index e4171dd..072f416 100644
--- a/cmd/txtar/txtar.go
+++ b/cmd/txtar/txtar.go
@@ -15,6 +15,10 @@
// from stdin and extract all of its files to corresponding locations relative
// to the current, writing the archive's comment to stdout.
//
+// Archive files are by default extracted only to the current directory or its
+// subdirectories. To allow extracting outside the current directory, use the
+// --unsafe flag.
+//
// Shell variables in paths are expanded (using os.Expand) if the corresponding
// variable is set in the process environment. When writing an archive, the
// variables (before expansion) are preserved in the archived paths.
@@ -44,6 +48,7 @@
var (
extractFlag = flag.Bool("extract", false, "if true, extract files from the archive instead of writing to it")
+ unsafeFlag = flag.Bool("unsafe", false, "allow extraction of files outside the current directory")
)
func init() {
@@ -69,7 +74,7 @@
}
if err != nil {
- fmt.Fprintln(os.Stderr, err)
+ fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
}
@@ -81,6 +86,30 @@
}
ar := txtar.Parse(b)
+
+ if !*unsafeFlag {
+ // Check that no files are extracted outside the current directory
+ wd, err := os.Getwd()
+ if err != nil {
+ return err
+ }
+ // Add trailing separator to terminate wd.
+ // This prevents extracting to outside paths which prefix wd,
+ // e.g. extracting to /home/foobar when wd is /home/foo
+ if !strings.HasSuffix(wd, string(filepath.Separator)) {
+ wd += string(filepath.Separator)
+ }
+
+ for _, f := range ar.Files {
+ fileName := filepath.Clean(expand(f.Name))
+
+ if strings.HasPrefix(fileName, "..") ||
+ (filepath.IsAbs(fileName) && !strings.HasPrefix(fileName, wd)) {
+ return fmt.Errorf("file path '%s' is outside the current directory", f.Name)
+ }
+ }
+ }
+
for _, f := range ar.Files {
fileName := filepath.FromSlash(path.Clean(expand(f.Name)))
if err := os.MkdirAll(filepath.Dir(fileName), 0777); err != nil {
diff --git a/cmd/txtar/txtar_test.go b/cmd/txtar/txtar_test.go
index 4130336..ea9f2bf 100644
--- a/cmd/txtar/txtar_test.go
+++ b/cmd/txtar/txtar_test.go
@@ -50,35 +50,93 @@
if err := os.Mkdir(dir, 0755); err != nil {
t.Fatal(err)
}
- if out := txtar(t, dir, testdata, "--extract"); out != comment {
+ if out, err := txtar(t, dir, testdata, "--extract"); err != nil {
+ t.Fatal(err)
+ } else if out != comment {
t.Fatalf("txtar --extract: stdout:\n%s\nwant:\n%s", out, comment)
}
// Now, re-archive its contents explicitly and ensure that the result matches
// the original.
args := []string{"one.txt", "dir", "$SPECIAL_LOCATION"}
- if out := txtar(t, dir, comment, args...); out != testdata {
+ if out, err := txtar(t, dir, comment, args...); err != nil {
+ t.Fatal(err)
+ } else if out != testdata {
t.Fatalf("txtar %s: archive:\n%s\n\nwant:\n%s", strings.Join(args, " "), out, testdata)
}
}
+func TestUnsafePaths(t *testing.T) {
+ // Set up temporary directories for test archives.
+ parentDir, err := ioutil.TempDir("", "txtar")
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer os.RemoveAll(parentDir)
+ dir := filepath.Join(parentDir, "dir")
+ if err := os.Mkdir(dir, 0755); err != nil {
+ t.Fatal(err)
+ }
+
+ // Test --unsafe option for both absolute and relative paths
+ testcases := []struct{ name, path string }{
+ {"Absolute", filepath.Join(parentDir, "dirSpecial")},
+ {"Relative", "../special"},
+ }
+
+ for _, tc := range testcases {
+ t.Run(tc.name, func(t *testing.T) {
+ // Set SPECIAL_LOCATION outside the current directory
+ t.Setenv("SPECIAL_LOCATION", tc.path)
+
+ // Expand the testdata archive into a temporary directory.
+
+ // Should fail without the --unsafe flag
+ if _, err := txtar(t, dir, testdata, "--extract"); err == nil {
+ t.Fatalf("txtar --extract: extracts to unsafe paths")
+ }
+
+ // Should allow paths outside the current dir with the --unsafe flags
+ out, err := txtar(t, dir, testdata, "--extract", "--unsafe")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if out != comment {
+ t.Fatalf("txtar --extract --unsafe: stdout:\n%s\nwant:\n%s", out, comment)
+ }
+
+ // Now, re-archive its contents explicitly and ensure that the result matches
+ // the original.
+ args := []string{"one.txt", "dir", "$SPECIAL_LOCATION"}
+ out, err = txtar(t, dir, comment, args...)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if out != testdata {
+ t.Fatalf("txtar %s: archive:\n%s\n\nwant:\n%s", strings.Join(args, " "), out, testdata)
+ }
+ })
+ }
+}
+
// txtar runs the txtar command in the given directory with the given input and
// arguments.
-func txtar(t *testing.T, dir, input string, args ...string) string {
+func txtar(t *testing.T, dir, input string, args ...string) (string, error) {
t.Helper()
cmd := exec.Command(txtarName(t), args...)
cmd.Dir = dir
+ cmd.Env = append(os.Environ(), "PWD="+dir)
cmd.Stdin = strings.NewReader(input)
stderr := new(strings.Builder)
cmd.Stderr = stderr
out, err := cmd.Output()
if err != nil {
- t.Fatalf("%s: %v\n%s", strings.Join(cmd.Args, " "), err, stderr)
+ return "", fmt.Errorf("%s: %v\n%s", strings.Join(cmd.Args, " "), err, stderr)
}
if stderr.String() != "" {
t.Logf("OK: %s\n%s", strings.Join(cmd.Args, " "), stderr)
}
- return string(out)
+ return string(out), nil
}
var txtarBin struct {
