Rocket: Overly strict FromSegments implementation for PathBuf

[Noughmad]

In the interest of safety, the implementation of FromSegments for PathBuf (code) rejects all segments that start with a dot (.). I believe this is overly strict, since

1. The dot-dot case (a filename of exactly ..) is already handled.
2. A single dot segment (exactly .) could also be valid, and just skipped.
3. Lots of hidden files start with ., such as .gitignore and other config files.

In my use case I only require the third case, as I am trying to create a file browser, where the path is specified as <path..> and path: PathBuf. However, such a route cannot browse a file with a name starting with a dot, and I get a Failed to parse 'path': BadStart('.') error message.

I can reproduce this behavior on version 0.3.6 on ArchLinux, but looking at the code I can see it is present in master as well.

[zxvfxwing]

Hi,

I have also encountered this behavior when I wanted to accept dotfiles (as you say) in my routes.
@SergioBenitez answered me on matrix channel / IRC bridge after exposing my problem.
You can read the entire conversation here.

So I made a workaround (used in one of my personal project) which is certainly not perfect but works.

Use case:

#[get("/<unsafe_p..>")]
pub fn dotfiles(unsafe_p: UnsafePBuf) -> ... {
    let dotpath = unsafe_p.path();
    /* Do wathever you want with your path */
}

Hopefully, my answer met your expectations.

[Noughmad]

Thank you @zxvfxwing , I was about to make a similar workaround myself, and you just saved me the effort.

That said, it would still be great if such a change could be made in Rocket itself. It looks like there is interest for it, and I can submit a PR. The question is if @SergioBenitez would accept it, and which cases may be allowed. I'm not really a web dev, so I'm not familiar with all the forms of path traversal attacks.

[SergioBenitez]

The danger is that users will unknowingly leak hidden files if this change is made. I'd like to keep the safe-by-default behavior while also allowing users to disable this on a per-case basis.

In https://github.com/SergioBenitez/Rocket/issues/239#issuecomment-288869202 I sketch out an API for mounting a "StaticFiles" handler. To resolve this issue, that handler could accept options on creation to allow this behavior:

rocket::ignite()
    .mount("/path_to_serve_on", StaticFiles::from("/path_to_serve_from").allow_hidden());

I've already laid out all of the ground work to enable the implementation of something like StaticFiles, and I've implemented a very basic version of StaticFiles. Once my time frees up a bit, I'll polish the implementation and merge it in.

If this sounds like a good plan moving forward @Noughmad, let's close this and track progress in #239. (Edit: Whoops. Didn't mean to close it myself!)

P.S: Complementary to this, we could have the StaticFiles handler deny hidden files by default but have PathBuf allow them. We'd then advocate for _only_ using StaticFiles to serve static files with the safe-by-default behavior. PathBuf would be relegated to special cases, protecting against path traversal attacks but nothing else.

[Noughmad]

Serving static files is not good for me, as I want to show other things in addition to the file content.

However, as I said I the workaround meantioned above works for me, and is not overly complicated. So unless you wish to change the current implementation, feel free to close the issue.

[SergioBenitez]

@Noughmad The plan in my previous comment covers your use-case as well by relaxing the constraints on PathBuf. I'll close this out and track in #239.