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.

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

path/filepath: reject Windows volume names containing .. components #58451

Open
neild opened this issue Feb 9, 2023 · 1 comment
Open

path/filepath: reject Windows volume names containing .. components #58451

neild opened this issue Feb 9, 2023 · 1 comment
Assignees
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Milestone

Comments

@neild
Copy link
Contributor

neild commented Feb 9, 2023

(New issue branching off discussion on #56336.)

https://go.dev/cl/444280 changed Windows volume name detection to be more accurate, correctly detecting a number of previously unrecognized volume name prefixes such as \\.\?\UNC\a\b. (Yes, that's a valid volume name.)

This change also results in us treating some invalid filenames as containing a prefix. For example, \\ is not a valid Windows filename, but we now consider it to have a \\ volume name on the grounds that preserving the semantically-significant \\ prefix is better than ignoring it.

A consequence of this change is that on Windows filepath.Clean can produce a path that can contain .. elements. For example, filepath.Clean("//../../a") is \\..\..\a. I'm not sure if \\..\.. is a valid Windows volume name or not, but a consequence of this is that code like

// Incorrect: Please do not write this.
path = filepath.Join(root, filepath.Clean("/" + slashSeparatedPath))

can produce a result that escapes the intended root when slashSeparatedPath is something like /../../foo.

While this code should be written as something like

// Using path.Clean rather than filepath.Clean avoids applying Windows volume semantics to the input path.
path = filepath.Join(root, path.Clean("/" + slashSeparatedPath))

we can still do a better job of defending against vulnerabilities caused by this misuse of filepath.Clean by rejecting volume names that contain .. as a component. So far as I know, .. never appears in Windows volume names.

Concretely:

filepath.Clean(`\\..\..\a`) = `\a`
filepath.VolumeName(`\\..\..\a`) = ``
@bcmills
Copy link
Contributor

bcmills commented Feb 10, 2023

(CC @golang/windows)

@bcmills bcmills added this to the Backlog milestone Feb 10, 2023
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows
Projects
None yet
Development

No branches or pull requests

2 participants