Page 1 of 2

basename secure for path injection?

Posted: Wed Sep 17, 2008 4:34 pm
by alex.barylski
I have the following code snippet...

Code: Select all

 
$path = basename($_GET['path']);
$path = "/var/www/$path/index.html"
 
I believe it is secure because as I understand the most an attacker could do is add a single dot to indicate a file instead of path, so...

Code: Select all

$path = '/var/www/somefile.dat/index.html';
There are no files of interest where 'somefile.dat' is concerned...but can they travel outside of that directory anyway?

Re: basename secure for path injection?

Posted: Thu Sep 18, 2008 2:05 am
by sergeyba
if you want your code secure dont trust any user input that is used later in SQL,filenames or pretty much anything else.
all user code should be varified and escaped.

RULE: Always either escape and validate user input or and even better (if possible) put your own values.

specific for your question the answer is yes. because there are many unix/linux/windows commands that travel up and down the directory tree and my guess is that they can be used here too.

example:
$_GET['path'] = "../../etc/passwd";

now there are two option:
1. your file that contains all the login information to the linux machine will be replaced. (not likely, because first you must be root user and second i believe that apcache himself will block this action). - still doesnt worth the risk and can do some bad things with other files that the user may have permission to.

2. nothing. (most likely, the user will be blocked somewhere on the server, but this example is very exclussive, and other cases may have different result).

Re: basename secure for path injection?

Posted: Thu Sep 18, 2008 2:15 am
by sergeyba
if you want your code secure dont trust any user input that is used later in SQL,filenames or pretty much anything else.
all user code should be varified and escaped.

RULE: Always either escape and validate user input or and even better (if possible) put your own values.

specific for your question the answer is yes. because there are many unix/linux/windows commands that travel up and down the directory tree and my guess is that they can be used here too.

example:
$_GET['path'] = "../../etc/passwd";

now there are two option:
1. your file that contains all the login information to the linux machine will be replaced. (not likely, because first you must be root user and second i believe that apcache himself will block this action). - still doesnt worth the risk and can do some bad things with other files that the user may have permission to.

2. nothing. (most likely, the user will be blocked somewhere on the server, but this example is very exclussive, and other cases may have different result).

Re: basename secure for path injection?

Posted: Thu Sep 18, 2008 3:12 am
by alex.barylski
Thanks for the reply...

1. The variable is not being used in a SQL statement but file inclusion.
2. I use basename() to I assume remove any harmful characters -- which is really what my question was.

Does basename() do the trick of eliminating any risk when used in a file inclusion?

I know I could just strip the characters but basename() if it does the trick and drops anything but a single period does this not work as well?

Re: basename secure for path injection?

Posted: Thu Sep 18, 2008 6:16 am
by Maugrim_The_Reaper
I'd suggest dropping basename as a security filter and falling back on good old regular expressions. The added flexibility would let you dictate valid path prefixes, strip invalid ending chars etc. Problem with basename is that it's not totally under your control - there'll always be a minimal risk it won't completely cover your expectations.

Re: basename secure for path injection?

Posted: Thu Sep 18, 2008 2:30 pm
by alex.barylski
^^^ Thats basically what I needed to hear...to justify using preg_replace. :)

Cheers :)

Re: basename secure for path injection?

Posted: Fri Sep 19, 2008 1:40 am
by matthijs
But if you can't trust basename, why should you trust preg_replace?

Besides that, if you use preg_replace you now have to trust 2 things: preg_replace itself AND your own ability to write a good filter statement for it.

Re: basename secure for path injection?

Posted: Fri Sep 19, 2008 3:19 am
by Maugrim_The_Reaper
It's not simply trust - basename() has a lot going for it but it's it doesn't necessarily match the user's expectations. If anything it's used a lot out of pure blind habit. Does it work? Within a specific set of scenarios, yes. Should people use it? Not unless it's backed by a lot of unit testing to reconcile your expectations with its operation. There are hundreds of security holes out there which depend on basename being used because it's own quirks and effects can be used to manipulate inputs indirectly.

I suggested PCRE because it can be more restrictive and it encourages safer whitelist thinking. Yes, you need to know regular expressions (presumably Hockey does ;)).

Re: basename secure for path injection?

Posted: Fri Sep 19, 2008 3:55 am
by matthijs
Interesting Maugrim. But could you be more specific or give some examples? If one would read the php manual, the basename() function seems pretty clear.

And in what ways would PCRE be more restrictive?

Re: basename secure for path injection?

Posted: Sat Sep 20, 2008 4:44 pm
by alex.barylski
Yes, you need to know regular expressions (presumably Hockey does ).
Enough to get by usually...haha

Really it's a simple matter of stripping any characters other than a-z and underscore...

Re: basename secure for path injection?

Posted: Sat Sep 20, 2008 6:39 pm
by josh
basename() will stop them from moving outside of a given directory or prefixing the path with extra subdirectories. as long as theres nothing else in that directory they can include that would be harmful, you should be set. I would at least add an if( file_exists()) in there too. Advantage over preg_replace is you don't have to update the regex when things change, disadvantage is you could forget you used basename or another developer could accidentally put something in the directory inadvertently later down the road

Re: basename secure for path injection?

Posted: Mon Sep 22, 2008 1:15 am
by matthijs
I'd still be interested to know in which ways basename() would not work (according to Maugrim)

Re: basename secure for path injection?

Posted: Mon Sep 22, 2008 9:34 am
by Maugrim_The_Reaper
Some simple examples of unintended usage:

1. You're dealing with files containing UTF-8 characters which in some PHP5 versions get dropped when they are the leading characters (adding an ASCII char first solves this). This might be used to conceal a targeted file from cursory detection against a blacklist.
2. You're applying basename() to a path which will later be printed to screen. You could craft a path name which passes cursory validation but the filename extracted is crafted javascript for XSS (same could be used for SQL Injection).

Both are common flaws in the wild.

As I said before, these are largely down to basename() being used habitually in validation/filtering of paths. Deep down though it's just a string manipulator. If it's not backed up with the proper safeguards like whitelists, file existence checks, etc. it poses a risk.

The difference with preg_match is that you can create a more inclusive regular expression with a far more limited acceptance criteria for filenames. That's why I don't like basename - it's an open door without additional support, and usually there is no additional filtering/validation because people think it's miraculously invulnerable for some reason.

Re: basename secure for path injection?

Posted: Mon Sep 22, 2008 11:04 am
by josh
Maugrim_The_Reaper wrote: 2. You're applying basename() to a path which will later be printed to screen. You could craft a path name which passes cursory validation but the filename extracted is crafted javascript for XSS (same could be used for SQL Injection).
the definition of basename is that it returns a valid file name, it never claims to escape HTML output, but this is a good thing to be aware of.

I'd be interested to see a use case showing how you'd exploit basename() with utf-8 strings

Basename is a filter, not a whitelist. Combining security methods will always lead to more security, that doesn't make any individual technique insecure in itself.
Deep down though it's just a string manipulator.
then what is regex? lol

Re: basename secure for path injection?

Posted: Mon Sep 22, 2008 11:54 am
by Maugrim_The_Reaper
I don't think you're interpreting me quite right ;).

I wasn't referring to basename() being insecure in and of itself - merely that using it in isolation without supporting measures does in some circumstances lead to vulnerabilities. It's more a comment on the level of faith some developers put in native PHP functions, or rather their misinterpretation of those functions. basename() doesn't return a filename, it returns a string that passes for a valid system filename. That's a subtle distinction. Unfortunately some developers don't get the distinction and assume the returned string is what they consider a "safe filename", i.e. something which requires no other validation. This line of thinking drives a whole range of XSS and other attacks where filename fragments are displayed to users, admins or sent in SQL statements. All you need is a weak escaping mechanism (and God knows how common that is!) and you have yourself a vulnerability.

That's the basis of why I don't like basename() being suggested as a standalone, isolated, solution. Generally it *must* have additional mechanisms to screen the result for whatever specific use case the filename is wanted for. My point being that many people just don't do that.

Me, I would admit to being somewhat extreme about avoiding it as part of a validation/filtering layer. Since it touches on behavioural factors in security prevention, I find the extra work in writing a regular expression that specifically limits what I, personally, intrepret as an allowable filename is a more focused approach less likely to fall afoul of the same mistakes. I'd go so far as to say I scan for basename() use as a standard XSS/SI/Injection vector.
then what is regex?
You probably (hopefully) get my point now ;). Regex can be written to be far more restrictive than basename() alone. You could argue for basename(), then regex - but I just drop the basename() use to limit the number of steps (and discourage any of the afore mentioned basename() blind dependence from other maintaining the source).