4
\$\begingroup\$

As with other inputs, file paths should be validated before thay could be passed to operating system interfaces. Since chroot(2) has a bit more than some portability issues, oftentimes it is desireable to use a path-validating function.

The general idea, is that no path component should be .., and that if we chdir(2) to the path in question, we must make sure it doesn't begin with a <slash> ("/").

Also, for some application, files or directories with certain prefix (say <dot> (".")) should be outright rejected.

Here's my example of PHP code that does path-validation. I'd like some expertized opinion from the members of the site.

<?php
 function antitraversal($s)
 {
   if( !is_string($s) ) die("Invalid argument: $s");

   $components = explode("/", $s);
   foreach($components as $component) {
     if( $component[0] === "." ) die("Invalid argument: $s");
   }

   if( $s[0] === "/" ) { return ".$s"; } else { return $s; };
 }

For simplicity, we'll assume the script runs in Unix-like environments.

\$\endgroup\$
4
\$\begingroup\$

Security

This can be bypassed by /\...

You are currently checking if / is followed by . and reject the input if that is the case. But as you can see, it is possible to traverse with different sequences (null bytes would be another candidate for bypasses).

It would be a bit more secure if you checked for ... To really prevent directory traversal, you should check if the resolved path is inside the desired directory.

Misc

  • dieing inside a function is always a bad idea, as you have no more control over the code. The calling function can't recover, the function can't be reused, etc.
  • You are vulnerable to XSS.
  • I would put each statement on it's own line (e.g. if/else on the same line is a bit difficult to read).
\$\endgroup\$
  • \$\begingroup\$ I've always wanted to know, how to know if we're in the desired directory, if all we have is a yet-to-be validated path... This seem to be a chicken-egg problem. Also, can you explain a bit how to "traverse with different sequence" and "null bytes would be another ..." since I've always had the impression you can't pass anything after the NUL byte into the operating system. \$\endgroup\$ – DannyNiu Feb 24 '18 at 1:35
0
\$\begingroup\$

From what I can tell (which I can because I've intended it), the function is very limited. It'll be a problem unless there's the separation of the concern.

First of all, the resulting string can only be passed to operating system interfaces such as chdir(2) or opendir(3), because these functions won't unquote your string like shell does.

Second, if you chdir to some directory then open a file in it, you need to further check the "filename" doesn't contain any slash - that is, you need a second function.

Third, as others have suggested about "\" (back-slash), even though it may not be an issue in system calls, it's still problematic cross-platform. If you want to make the code "cygwin-friendly", you can call strtr to replace all occurences with "/" (forward-slash).

And finally, we need to get rid of $s when we die, it's an XSS hole.

Here's a version I'd suggest as improvement, and I request comment from the community:

<?php
 // called on directory paths. 
 function antitraversal($s)
 {
   if( !is_string($s) ) die("Invalid argument!");
   $s = strtr($s, "\\", "/"); // cygwin-safe. 

   $components = explode("/", $s);
   foreach($components as $component) {
     if( $component[0] === "." ) die("Invalid argument!");
   }

   return ($s[0]==="/") ? ".$s" : $s;
 }

 // called on file names. 
 function antidir($s)
 {
   if( !is_string($s) ) die("Invalid argument!");
   $s = strtr($s, "\\", "/"); // cygwin-safe. 

   if( strchr($s, "/") !== false ) die("Invalid argument!");
   if( $s[0] === "." ) die("Invalid argument!");
   if( !is_file($s) ) die("Invliad argument!");

   return $s;
 }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.