views:

480

answers:

2

I'm a beginner with PHP security issues. Someone reported a security issue with my code, but never gave details.

Below is a condensed version of the code. I'm using the JQuery AJAX function to send some data to the server where it's used by the PHP scandir() function.

Do you see any security issues?

HTML Document

 <script 
   src="http://ajax.googleapis.com/ajax/libs/jquery/1.3.2/jquery.min.js"&gt;
 </script>
 <script>
$.ajax({
    type: "POST",
    url: "example.php",
    cache: false,
    data: {
     foo: "images",
     bar: "something else",
            },
    success: function(html){
    alert(html);
    }
});
</script>

PHP Document

<?php

$foo = filter_var($_POST["foo"], FILTER_SANITIZE_STRING);
$bar = filter_var($_POST["bar"], FILTER_SANITIZE_STRING);

$foo = scandir($foo);
$bar = (explode(",",$bar));

foreach ($foo as $a) {
    echo "<img src='$foo/$a'>";
}

?>
+1  A: 

You could potentially obtain a listing of any files on your filesystem by posting a malicious value called 'foo' - if a value '/' was sent, you would be able to see in the HTML source all the files in your filesystem root. This might not always work because of things like open_basedir restrictions, but it still isn't a good idea.

A possible solution would be to sanitise the 'foo' parameter first - e.g. only allow certain characters, or you could only allow 'foo' values which exist within a whitelist of allowed directory names, e.g.:

$foo = filter_var($_POST["foo"], FILTER_SANITIZE_STRING);

if (!in_array($foo, array('allowed_dir1', 'allowed_dir2'))) {
    //not valid, not in whitelist
}

//or

if (!preg_match('/^[a-zA-Z0-9+]+$/', $foo)) {
   //not valid, contains more than just letters and numbers
}
Tom Haigh
"This way, images can be sourced from anywhere" - You really don't want to give any remote user the ability to look at any file "anywhere" on your machine. Even if only image files. And remember, just because a file has an image extension, such as "jpg", it doesn't mean that it contains image data. A malicious attacker may have left code there via some other vulnerability. I strongly recommend you rethink your design and requirements.
Cheekysoft
+3  A: 

Yes, I can enumerate every file on your machine. And possibly other machines that your server has access to via fopen wrappers. This information can help identify other vulnerable software running or sensitive files, in order to help me target further attacks.

FILTER_SANITIZE_STRING is designed to protect HTML, not file path references, so it really isn't helping you here. You should only use that as an output filter to the generated HTML and not when using a backend fuction.

You want to restrict uses of dangerous path tokens like :, @, .., \ and /. But do this by whitelisting the characters or values that you consider safe, as in Tom Haigh's example code.

Cheekysoft