From 12cd55c062d6062a64076cb37f12ab7646df1be7 Mon Sep 17 00:00:00 2001 From: Constantin Konstantinidis Date: Wed, 25 Dec 2019 17:24:07 +0100 Subject: [PATCH] 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 Reviewed-by: Robert Griesemer Run-TryBot: Emmanuel Odeke TryBot-Result: Gobot Gobot --- src/io/ioutil/tempfile.go | 19 ++++++-- src/io/ioutil/tempfile_test.go | 79 ++++++++++++++++++++++++++++++++++ src/os/os_test.go | 2 +- 3 files changed, 96 insertions(+), 4 deletions(-) diff --git a/src/io/ioutil/tempfile.go b/src/io/ioutil/tempfile.go index 3aa23c5f01..af7c6fd7c1 100644 --- 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 698ebabee9..469d2c98b3 100644 --- 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 278c19e44b..802ecc4e49 100644 --- 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) }