Question

I've built my filebrowser so far but it's pretty insecure. For example when you insert "?dir=../" or "?dir=../../../" you can view files you shouldn't be able to. My code:

<?php
    if(isset($_GET['dir']) && $_GET['dir'] !== "../") {$dir = $basedir.$_GET['dir']."/";} else {$dir = $basedir;}
    if ($handle = opendir($dir)) {
        if(isset($_GET['dir']) && $_GET['dir'] !== "") {echo "<a href='?dir='>back to root</a>";}
        while (false !== ($file = readdir($handle))) {
            if ($file != "." && $file != "..") {
                if(filetype($dir . $file) == "file") {$filetype = what_suffix($file);} else {$filetype = "-";}
                if(filetype($dir . $file) == "file") {$filesize = round( filesize($dir . $file)/1024/1024 ,2)." MB";} else {$filesize = "-";}
                if(filetype($dir . $file) == "file") {$filedate = date("d.m.Y H:i:s", filectime($dir.$file));} else {$filedate = "-";}
                if(filetype($dir . $file) == "file") {
                echo(
                    "<tr>
                        <td><a href='?dir=".@$_GET['dir']."&file=".$file."'>".$file."</td>
                        <td>".$filetype."</td>
                        <td>".$filesize."</td>
                        <td>".$filedate."</td>
                    </tr>"
                ); } else {
                echo(
                    "<tr>
                        <td><a href='?dir=".@$_GET['dir']."/".$file."'>".$file."</td>
                        <td>-</td>
                        <td>-</td>
                        <td>-</td>
                    </tr>"
                ); }


            }
        }
        closedir($handle);
    }
?>

Is there a way I could restrict that? (I guess I just don't see the wood for the trees)

Was it helpful?

Solution

Here is a basic security check I have used for such things.

function isSecurityViolation($dir) {
    if (preg_match('#https?://#i', $dir)) {
        return true;
    }

    // no directory traversal attempts
    if (strpos($dir, '../') !== false) {
        return true;
    }

    if (strpos($dir, "\x00") !== false) {
        return true;
    }

    if (strpos($dir, '%00') !== false) {
        return true;
    }

    return false;
}

It checks for the following violations:

  • Attempting to open an http or https URL
  • Directory traversal using ..
  • Null character checks which can allow malicious urls to pass certain functions

Somewhere towards the beginning of your script, you could call:

if (isSecurityViolation($dir)) {
    die('No soup for you.');
}
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top