-
Notifications
You must be signed in to change notification settings - Fork 6
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
Converting User-Created Paths is a Hazard #22
Comments
Hm, it sounds like the discussion in the issue for Rust was suggesting some sort of My first thought is to support a strict series of methods to push paths, join paths, and instantiate paths. Each of these can fail and thereby return a result. Thoughts? use typed_path::{Utf8Path, Utf8UnixEncoding, Utf8WindowsEncoding};
fn main() {
// If the path itself contains something invalid ??
let windows_path = Utf8Path::<Utf8UnixEncoding>::new_strict("/tmp/d:/foo").unwrap();
dbg!(windows_path);
// If the conversion would lead to pushing absolute paths, this would fail
let unix_path = windows_path.with_strict_encoding::<Utf8WindowsEncoding>().unwrap();
dbg!(unix_path);
// Imagine there would also be these methods
// unix_path.push_strict("...").unwrap();
// unix_path.join_strict("...").unwrap();
} |
#19 looks like its covering the more general case of this issue. Yes, the issue is with an absolute or prefix path overwriting the entire path. In many contexts it is a security hazard, like pushing url components to a path in a static file server or creating an output path in a file archive extractor. See tower-rs/tower-http#204 for a good example of the kinds of issues it can cause. However, I opened this issue to cover, specifically, the conversions between Unix and Windows path types with the For my use case, I am interested in converting an untrusted Unix-style path into a Windows-style path. I want a
I think that is a good solution for the general case. I think the wrapper approach in #20 is overkill. I think this will also fix my use case as well.
The issue is a slightly deeper than that. ":" is valid within file names on Unix, but not on Windows; the above path is a valid Unix path. On Windows, ":" is usually used for drive prefixes. This means converting this path will completely change its meaning, with no way to properly round-trip the path. As a side note, since I haven't tested, how does this crate handle the other banned Windows file name chars? I'm not sure if its a good idea to keep |
I'll take a stab at implementing this soon. Possibly today or this weekend if I have time. Will share the PR and let you take a look to see if it addresses what you'd expect.
For my personal use case, I want a conversion that does not require unwrapping or handling an error, but I can see why that may not be desired by many. As part of the above, I can offer a Alternatively, given this library is still being developed and not at a 1.0 release, I could rename the pre-existing encoding conversions functions to something like |
@nathaniel-daniel can you check if version |
Thanks, that works from my tests. I'll open a new issue if I find any more problems. |
This is related to #19, but this focuses more on usage within the library itself instead of from the outside.
See rust-lang/rust#59726 for more info.
Essentially, pushing a non-relative path to a path can cause the path to be replaced. This can be hazardous when a user controls the input path, like a webserver.
Here's a simple example:
Outputs:
Converting between path formats should not use join to build paths, or at least return some kind of error instead of overwriting the prior path.
The text was updated successfully, but these errors were encountered: