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

Properly evaluate static serving wrt directory traversal attacks #54

Open
steveklabnik opened this issue Oct 15, 2017 · 3 comments
Open

Comments

@steveklabnik
Copy link
Owner

We serve static files, and check for . and .. in the path. This is probably not enough, but needs to be researched more.

@kardeiz
Copy link
Contributor

kardeiz commented Oct 17, 2017

It's not applicable verbatim, but Rocket's security checks for paths might be a good starting point: https://github.com/SergioBenitez/Rocket/blob/master/lib/src/request/param.rs#L328.

@scurest
Copy link
Contributor

scurest commented Mar 23, 2018

Note that empty components in a path are ignored: /, //, and /////// all name the same directory.

With that in mind, a client can currently access any file the server process can access on the host machine using a path like //etc/passwd because the server only truncates a single leading / from the request URI's path.

simple-server/src/lib.rs

Lines 349 to 350 in 10103f5

// the uri always includes a leading /, which means that join will over-write the static directory...
let fs_path = self.static_directory.join(&fs_path[1..]);

Example

$ curl -vs http://127.0.0.1:7878//etc/passwd
root:x:0:0:root:/root:/bin/bash
...

Possible alternative strategy is

  1. ensure the static directory path is absolute
  2. canonicalize the static directory path
  3. join the uri path to the static directory path
  4. canonicalize the result
  5. check that the result starts_with the static directory path

@scurest
Copy link
Contributor

scurest commented Mar 23, 2018

Or how does this look to mitigate it? https://github.com/scurest/simple-server/commit/3ed6abe1a61e18f6dfdf3484902945d2a1e23699

We trim the single leading slash, put the rest into a path and iterate over its components, pushing the normals ones onto static_directory, but counting any non-normal component as a transversal attack.

edit: can make it even simpler. https://github.com/scurest/simple-server/commit/d2ccf63192e1904185913c03e493d59f5e6d8e62

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants