simple-server icon indicating copy to clipboard operation
simple-server copied to clipboard

Properly evaluate static serving wrt directory traversal attacks

Open steveklabnik opened this issue 8 years ago • 3 comments

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

steveklabnik avatar Oct 15 '17 21:10 steveklabnik

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.

kardeiz avatar Oct 17 '17 13:10 kardeiz

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.

https://github.com/steveklabnik/simple-server/blob/10103f59775591a470dc4d0275cfe54e635e9a5c/src/lib.rs#L349-L350

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 avatar Mar 23 '18 16:03 scurest

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

scurest avatar Mar 23 '18 20:03 scurest