diff options
author | Damien Neil <dneil@google.com> | 2022-12-12 16:43:37 -0800 |
---|---|---|
committer | Michael Pratt <mpratt@google.com> | 2023-02-14 17:25:26 +0000 |
commit | 3345ddca41f00f9ed6fc3c1a36f6e2bede02d7ff (patch) | |
tree | e76136b3f00e9f33a2d86312f14a1b970e417649 | |
parent | f69dbb6dfa8af7a32a47070d71b82fc4ed4b9d2a (diff) | |
download | go-git-3345ddca41f00f9ed6fc3c1a36f6e2bede02d7ff.tar.gz |
[release-branch.go1.19] path/filepath: do not Clean("a/../c:/b") into c:\b on Windows
Do not permit Clean to convert a relative path into one starting
with a drive reference. This change causes Clean to insert a .
path element at the start of a path when the original path does not
start with a volume name, and the first path element would contain
a colon.
This may introduce a spurious but harmless . path element under
some circumstances. For example, Clean("a/../b:/../c") becomes `.\c`.
This reverts CL 401595, since the change here supersedes the one
in that CL.
Thanks to RyotaK (https://twitter.com/ryotkak) for reporting this issue.
Updates #57274
Fixes #57275
Fixes CVE-2022-41722
Change-Id: I837446285a03aa74c79d7642720e01f354c2ca17
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1675249
Reviewed-by: Roland Shoemaker <bracewell@google.com>
Run-TryBot: Damien Neil <dneil@google.com>
Reviewed-by: Julie Qiu <julieqiu@google.com>
TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com>
(cherry picked from commit 780dfa043ff5192c37de0d6fd1053a66b2b9f378)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728206
Reviewed-by: Damien Neil <dneil@google.com>
Run-TryBot: Roland Shoemaker <bracewell@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/468115
Reviewed-by: Than McIntosh <thanm@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Bypass: Michael Pratt <mpratt@google.com>
-rw-r--r-- | src/path/filepath/path.go | 27 | ||||
-rw-r--r-- | src/path/filepath/path_test.go | 7 | ||||
-rw-r--r-- | src/path/filepath/path_windows_test.go | 2 |
3 files changed, 22 insertions, 14 deletions
diff --git a/src/path/filepath/path.go b/src/path/filepath/path.go index de7a2c758b..9b1f5ed7c0 100644 --- a/src/path/filepath/path.go +++ b/src/path/filepath/path.go @@ -15,6 +15,7 @@ import ( "errors" "io/fs" "os" + "runtime" "sort" "strings" ) @@ -117,21 +118,9 @@ func Clean(path string) string { case os.IsPathSeparator(path[r]): // empty path element r++ - case path[r] == '.' && r+1 == n: + case path[r] == '.' && (r+1 == n || os.IsPathSeparator(path[r+1])): // . element r++ - case path[r] == '.' && os.IsPathSeparator(path[r+1]): - // ./ element - r++ - - for r < len(path) && os.IsPathSeparator(path[r]) { - r++ - } - if out.w == 0 && volumeNameLen(path[r:]) > 0 { - // When joining prefix "." and an absolute path on Windows, - // the prefix should not be removed. - out.append('.') - } case path[r] == '.' && path[r+1] == '.' && (r+2 == n || os.IsPathSeparator(path[r+2])): // .. element: remove to last separator r += 2 @@ -157,6 +146,18 @@ func Clean(path string) string { if rooted && out.w != 1 || !rooted && out.w != 0 { out.append(Separator) } + // If a ':' appears in the path element at the start of a Windows path, + // insert a .\ at the beginning to avoid converting relative paths + // like a/../c: into c:. + if runtime.GOOS == "windows" && out.w == 0 && out.volLen == 0 && r != 0 { + for i := r; i < n && !os.IsPathSeparator(path[i]); i++ { + if path[i] == ':' { + out.append('.') + out.append(Separator) + break + } + } + } // copy element for ; r < n && !os.IsPathSeparator(path[r]); r++ { out.append(path[r]) diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index a783d6be28..9a57920dd7 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -96,6 +96,13 @@ var wincleantests = []PathTest{ {`.\c:`, `.\c:`}, {`.\c:\foo`, `.\c:\foo`}, {`.\c:foo`, `.\c:foo`}, + + // Don't allow cleaning to move an element with a colon to the start of the path. + {`a/../c:`, `.\c:`}, + {`a\..\c:`, `.\c:`}, + {`a/../c:/a`, `.\c:\a`}, + {`a/../../c:`, `..\c:`}, + {`foo:bar`, `foo:bar`}, } func TestClean(t *testing.T) { diff --git a/src/path/filepath/path_windows_test.go b/src/path/filepath/path_windows_test.go index 9e6c0ec81d..857f4d5c7c 100644 --- a/src/path/filepath/path_windows_test.go +++ b/src/path/filepath/path_windows_test.go @@ -542,7 +542,7 @@ func TestIssue52476(t *testing.T) { }{ {`..\.`, `C:`, `..\C:`}, {`..`, `C:`, `..\C:`}, - {`.`, `:`, `:`}, + {`.`, `:`, `.\:`}, {`.`, `C:`, `.\C:`}, {`.`, `C:/a/b/../c`, `.\C:\a\c`}, {`.`, `\C:`, `.\C:`}, |