Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

path/filepath: Clean on some invalid Windows paths can lose .. components [1.21 backport] #61868

Closed
gopherbot opened this issue Aug 8, 2023 · 2 comments
Closed

path/filepath: Clean on some invalid Windows paths can lose .. components [1.21 backport] #61868

gopherbot opened this issue Aug 8, 2023 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone
Go1.21.1

Comments

@gopherbot
Copy link
Contributor

@neild requested issue #61866 to be considered for backport to the next 1.21 minor release.

@gopherbot please open backport issues. This is a bug with no good workarounds.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Aug 8, 2023
@gopherbot gopherbot mentioned this issue Aug 8, 2023
Closed
@gopherbot gopherbot added this to the Go1.21.1 milestone Aug 8, 2023
@heschi heschi added the CherryPickApproved Used during the release process for point releases label Aug 9, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Aug 9, 2023
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/519655 mentions this issue: [release-branch.go1.21] path/filepath: don't drop .. elements when cleaning invalid Windows paths

gopherbot pushed a commit that referenced this issue Aug 23, 2023
@neild @cagedmantis
[release-branch.go1.21] path/filepath: don't drop .. elements when cl…
45b98bf 
…eaning invalid Windows paths

Fix a bug where Clean could improperly drop .. elements from a
path on Windows, when the path contains elements containing a ':'.

For example, Clean("a/../b:/../../c") now correctly returns "..\c"
rather than "c".

For #61866.
Fixes #61868.

Change-Id: I97b0238953c183b2ce19ca89c14f26700008ea72
Reviewed-on: https://go-review.googlesource.com/c/go/+/517216
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Quim Muntal <[email protected]>
(cherry picked from commit 6e43407)
Reviewed-on: https://go-review.googlesource.com/c/go/+/519655
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>
@gopherbot
Copy link
Contributor Author

Closed by merging 45b98bf to release-branch.go1.21.

rcrozean pushed a commit to rcrozean/go that referenced this issue Dec 7, 2023
@neild @rcrozean
path/filepath: fix various issues in parsing Windows paths
daff43d 
# AWS EKS

Backported To: go-1.19.13-eks
Backported On: Tue, 07 Nov 2023
Backported By: [email protected]
Backported From: release-branch.go1.20
Source Commit: golang@46fb781 \
  golang@45b98bf \
  golang@6d0bf43

In addition to the CVE fix golang@46fb781,

golang@45b98bf &
golang@6d0bf43 were
cherry-picked to include expected functions and tests expected in the CVE fix commit.
Additionally, for the purpose of tests I added the line to api/go1.19.txt which is used for checking the
function calls exist as expected.

# Original Information

On Windows, A root local device path is a path which begins with
\\?\ or \??\.  A root local device path accesses the DosDevices
object directory, and permits access to any file or device on the
system. For example \??\C:\foo is equivalent to common C:\foo.

The Clean, IsAbs, IsLocal, and VolumeName functions did not
recognize root local device paths beginning with \??\.

Clean could convert a rooted path such as \a\..\??\b into
the root local device path \??\b. It will now convert this
path into .\??\b.

IsAbs now correctly reports paths beginning with \??\
as absolute.

IsLocal now correctly reports paths beginning with \??\
as non-local.

VolumeName now reports the \??\ prefix as a volume name.

Join(`\`, `??`, `b`) could convert a seemingly innocent
sequence of path elements into the root local device path
\??\b. It will now convert this to \.\??\b.

In addition, the IsLocal function did not correctly
detect reserved names in some cases:

  - reserved names followed by spaces, such as "COM1 ".
  - "COM" or "LPT" followed by a superscript 1, 2, or 3.

IsLocal now correctly reports these names as non-local.

For golang#63713
Fixes golang#63714
Fixes CVE-2023-45283
Fixes CVE-2023-45284

Change-Id: I446674a58977adfa54de7267d716ac23ab496c54
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2040691
Reviewed-by: Roland Shoemaker <[email protected]>
Reviewed-by: Tatiana Bradley <[email protected]>
Run-TryBot: Damien Neil <[email protected]>
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/2072597
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-on: https://go-review.googlesource.com/c/go/+/539276
Auto-Submit: Heschi Kreinick <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

commit: golang@6d0bf43

[release-branch.go1.21] path/filepath: don't drop .. elements when cleaning invalid Windows paths

Fix a bug where Clean could improperly drop .. elements from a
path on Windows, when the path contains elements containing a ':'.

For example, Clean("a/../b:/../../c") now correctly returns "..\c"
rather than "c".

For golang#61866.
Fixes golang#61868.

Change-Id: I97b0238953c183b2ce19ca89c14f26700008ea72
Reviewed-on: https://go-review.googlesource.com/c/go/+/517216
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Quim Muntal <[email protected]>
(cherry picked from commit 6e43407)
Reviewed-on: https://go-review.googlesource.com/c/go/+/519655
Reviewed-by: Damien Neil <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Run-TryBot: Dmitri Shuralyov <[email protected]>

commit: golang@45b98bf

path/filepath: add IsLocal

IsLocal reports whether a path lexically refers to a location
contained within the directory in which it is evaluated.
It identifies paths that are absolute, escape a directory
with ".." elements, and (on Windows) paths that reference
reserved device names.

For golang#56219.

Change-Id: I35edfa3ce77b40b8e66f1fc8e0ff73cfd06f2313
Reviewed-on: https://go-review.googlesource.com/c/go/+/449239
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Joseph Tsai <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Joedian Reid <[email protected]>
@golang golang locked and limited conversation to collaborators Aug 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Milestone
Go1.21.1
Development

No branches or pull requests
2 participants
@gopherbot @heschi