I've got a PHP script which allows uses to delete certain files in a directory by specifying the file name (via a drop down - but thats easy enough for someone malicious to change). I'm 'cleansing' the file name by doing the following
if(preg_match(/'/^[a-zA-Z0-9.]$/'/,$file)) {
# do stuff to this particular file
}
I'm fairly sure that should prevent anyone from getting up to anything nasty, but as the people here have vast amounts of knowledge more then I, I thought I'd ask -- is there a hole here, or will this keep the nasty away?
I'm not sure whether your regex will really help: There are valid characters outside that range that you won't be able to delete this way.
One thing that I would do is do a realpath() on the full final path, and check whether it still is a child of your allowed file path. That will prevent ../../ directory traversal attacks, even if they use some special characters. That should already provide fairly good security.
You could also additionally scan the directory using glob() and check the results to see whether the requested file is actually in there (that is impossible to circumvent even with the most sneaky directory traversal.)
If you want to be totally paranoid about this, you could use a different approach altogether: Don't transfer file names, but list indexes of a list that you specified before. If for example you show this list to the user and save it in a temporary text file or database record:
and then pass only the (random) ID of the text file or database record, and the number of the file you want to delete:
delete.php?list=xasdafdas&index=3
you should have a solution that is pretty invulnerable against any conceivable kind of injection and file name tampering.
You would have to store an individual list for every request, as the files can change.
maybe you should change your regexp in
if(preg_match(/'/^[^a-zA-Z0-9.]+$/'/,$file)) {
# do stuff to this particular file
}
If your filename is not an absolute path, and your always prefix the directory path
chdir(...); // change directory to that directory
// make use the $file is not contains '/'
$file = basename($file);
// check $file is not in list of files that you don't allow for delete
if ($file=='index.php' ...)
{
// do nothing
return false;
}
if (is_file($file))
{
unlink($file); // or other actions
}
That regexp will match pretty much anything. Did you mean /^[a-zA-Z0-9.]/ instead? That would still match something like ../../../etc/shadow though. /^[a-zA-Z0-9.]+$/ (or simply /^[\w\d.]+$/) is better, or you could just check the filename against the array you created the dropdown from.