io/ioutil: reject path separators in TempDir, TempFile pattern

Fixes #33920

Change-Id: I2351a1caa80c086ff5a8e02aad70d996be7aac35
Reviewed-on: https://go-review.googlesource.com/c/go/+/212597
Reviewed-by: Emmanuel Odeke <emm.odeke@gmail.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/io/ioutil/tempfile.go b/src/io/ioutil/tempfile.go
index 3aa23c5f0173a2b188cbfacea4e64884be77f418..af7c6fd7c1a2dd5a5ec2c31786eeed44409b9395 100644 (file)
--- a/src/io/ioutil/tempfile.go
+++ b/src/io/ioutil/tempfile.go
@@ -5,6 +5,7 @@
 package ioutil
 
 import (
+       "errors"
        "os"
        "path/filepath"
        "strconv"
@@ -52,7 +53,10 @@ func TempFile(dir, pattern string) (f *os.File, err error) {
                dir = os.TempDir()
        }
 
-       prefix, suffix := prefixAndSuffix(pattern)
+       prefix, suffix, err := prefixAndSuffix(pattern)
+       if err != nil {
+               return
+       }
 
        nconflict := 0
        for i := 0; i < 10000; i++ {
@@ -71,9 +75,15 @@ func TempFile(dir, pattern string) (f *os.File, err error) {
        return
 }
 
+var errPatternHasSeparator = errors.New("pattern contains path separator")
+
 // prefixAndSuffix splits pattern by the last wildcard "*", if applicable,
 // returning prefix as the part before "*" and suffix as the part after "*".
-func prefixAndSuffix(pattern string) (prefix, suffix string) {
+func prefixAndSuffix(pattern string) (prefix, suffix string, err error) {
+       if strings.ContainsRune(pattern, os.PathSeparator) {
+               err = errPatternHasSeparator
+               return
+       }
        if pos := strings.LastIndex(pattern, "*"); pos != -1 {
                prefix, suffix = pattern[:pos], pattern[pos+1:]
        } else {
@@ -96,7 +106,10 @@ func TempDir(dir, pattern string) (name string, err error) {
                dir = os.TempDir()
        }
 
-       prefix, suffix := prefixAndSuffix(pattern)
+       prefix, suffix, err := prefixAndSuffix(pattern)
+       if err != nil {
+               return
+       }
 
        nconflict := 0
        for i := 0; i < 10000; i++ {

diff --git a/src/io/ioutil/tempfile_test.go b/src/io/ioutil/tempfile_test.go
index 698ebabee9e3feda2c7fd3463d9ad889baf528a4..469d2c98b3dbdeefb65e444a2f999adf2c39e167 100644 (file)
--- a/src/io/ioutil/tempfile_test.go
+++ b/src/io/ioutil/tempfile_test.go
@@ -48,6 +48,48 @@ func TestTempFile_pattern(t *testing.T) {
        }
 }
 
+func TestTempFile_BadPattern(t *testing.T) {
+       tmpDir, err := TempDir("", t.Name())
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(tmpDir)
+
+       const sep = string(os.PathSeparator)
+       tests := []struct {
+               pattern string
+               wantErr bool
+       } {
+               {"ioutil*test", false},
+               {"ioutil_test*foo", false},
+               {"ioutil_test" + sep + "foo", true},
+               {"ioutil_test*" + sep + "foo", true},
+               {"ioutil_test" + sep + "*foo", true},
+               {sep + "ioutil_test" + sep + "*foo", true},
+               {"ioutil_test*foo" + sep, true},
+       }
+       for _, tt := range tests {
+               t.Run(tt.pattern, func(t *testing.T) {
+                       tmpfile, err := TempFile(tmpDir, tt.pattern)
+                       defer func() {
+                               if tmpfile != nil {
+                                       tmpfile.Close()
+                               }
+                       }()
+                       if tt.wantErr {
+                               if err == nil {
+                                       t.Errorf("Expected an error for pattern %q", tt.pattern)
+                               }
+                               if g, w := err, errPatternHasSeparator; g != w {
+                                       t.Errorf("Error mismatch: got %#v, want %#v for pattern %q", g, w, tt.pattern)
+                               }
+                       } else if err != nil {
+                               t.Errorf("Unexpected error %v for pattern %q", err, tt.pattern)
+                       }
+               })
+       }
+}
+
 func TestTempDir(t *testing.T) {
        name, err := TempDir("/_not_exists_", "foo")
        if name != "" || err == nil {
@@ -112,3 +154,40 @@ func TestTempDir_BadDir(t *testing.T) {
                t.Errorf("TempDir error = %#v; want PathError for path %q satisifying os.IsNotExist", err, badDir)
        }
 }
+
+func TestTempDir_BadPattern(t *testing.T) {
+       tmpDir, err := TempDir("", t.Name())
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(tmpDir)
+
+       const sep = string(os.PathSeparator)
+       tests := []struct {
+               pattern string
+               wantErr bool
+       } {
+               {"ioutil*test", false},
+               {"ioutil_test*foo", false},
+               {"ioutil_test" + sep + "foo", true},
+               {"ioutil_test*" + sep + "foo", true},
+               {"ioutil_test" + sep + "*foo", true},
+               {sep + "ioutil_test" + sep + "*foo", true},
+               {"ioutil_test*foo" + sep, true},
+       }
+       for _, tt := range tests {
+               t.Run(tt.pattern, func(t *testing.T) {
+                       _, err := TempDir(tmpDir, tt.pattern)
+                       if tt.wantErr {
+                               if err == nil {
+                                       t.Errorf("Expected an error for pattern %q", tt.pattern)
+                               }
+                               if g, w := err, errPatternHasSeparator; g != w {
+                                       t.Errorf("Error mismatch: got %#v, want %#v for pattern %q", g, w, tt.pattern)
+                               }
+                       } else if err != nil {
+                               t.Errorf("Unexpected error %v for pattern %q", err, tt.pattern)
+                       }
+               })
+       }
+}

diff --git a/src/os/os_test.go b/src/os/os_test.go
index 278c19e44be1bbd38c13f60cdb4005dee0bba5f0..802ecc4e498e6b90b0214301cb07f3635526f8f0 100644 (file)
--- a/src/os/os_test.go
+++ b/src/os/os_test.go
@@ -1785,7 +1785,7 @@ func TestAppend(t *testing.T) {
 
 func TestStatDirWithTrailingSlash(t *testing.T) {
        // Create new temporary directory and arrange to clean it up.
-       path, err := ioutil.TempDir("", "/_TestStatDirWithSlash_")
+       path, err := ioutil.TempDir("", "_TestStatDirWithSlash_")
        if err != nil {
                t.Fatalf("TempDir: %s", err)
        }