basename secure for path injection?

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

basename secure for path injection?

Post 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?
sergeyba
Forum Newbie
Posts: 3
Joined: Sat Sep 13, 2008 12:04 pm

Re: basename secure for path injection?

Post 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).
sergeyba
Forum Newbie
Posts: 3
Joined: Sat Sep 13, 2008 12:04 pm

Re: basename secure for path injection?

Post 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).
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: basename secure for path injection?

Post 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?
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Re: basename secure for path injection?

Post 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.
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: basename secure for path injection?

Post by alex.barylski »

^^^ Thats basically what I needed to hear...to justify using preg_replace. :)

Cheers :)
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Re: basename secure for path injection?

Post 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.
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Re: basename secure for path injection?

Post 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 ;)).
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Re: basename secure for path injection?

Post 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?
alex.barylski
DevNet Evangelist
Posts: 6267
Joined: Tue Dec 21, 2004 5:00 pm
Location: Winnipeg

Re: basename secure for path injection?

Post 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...
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: basename secure for path injection?

Post 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
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Re: basename secure for path injection?

Post by matthijs »

I'd still be interested to know in which ways basename() would not work (according to Maugrim)
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Re: basename secure for path injection?

Post 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.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: basename secure for path injection?

Post 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
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Re: basename secure for path injection?

Post 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).
Post Reply