-1

Would this be safe??

I would just like to hear if this "Cms" would be safe. So people cant do example.com/../../../systemfile.conf

My index.php

<?php

$url= rtrim($_GET['page'], '/');

echo $url;

include_once "sites/$url";

?>

My .htaccess

RewriteEngine On
options +FollowSymlinks
RewriteBase /
RewriteCond %{REQUEST_FILENAME} -f [OR]
RewriteCond %{REQUEST_FILENAME} -l [OR]
RewriteCond %{REQUEST_FILENAME} -d
RewriteRule ^.* - [L]
RewriteRule ^(.+)/? index.php?page=$1 [L,QSA]
AstroCB
  • 12,337
  • 20
  • 57
  • 73
KaareZ
  • 615
  • 2
  • 10
  • 22
  • No, this is not secure: `index.php?page=../../../systemfile.conf` – Gumbo Mar 19 '14 at 16:35
  • [Preventing Directory Traversal in PHP but allowing paths](http://stackoverflow.com/q/4205141/53114) might answer your question. – Gumbo Mar 19 '14 at 20:33

2 Answers2

0
include_once "sites/$url";

Should be changed to...

$inc='sites/' . realpath($url);
if (!is_readable($inc)) {
      header("HTTP/1.0 404 Not Found");
      exit;
}
include($inc);

as a minimum

symcbean
  • 47,736
  • 6
  • 59
  • 94
  • -1 This won’t work. `realpath` already returns the canonical absolute path or `false`. – Gumbo Mar 19 '14 at 20:31
  • Ahh, true, I didn't say I'd *tested* it :) How about ...$stub=dirname(__FILE__).'/sites/'; $inc=realpath($stub.$url); if (substr($inc, 0, strlen($stub))!=$stub || ! is_radable($inc)) { – symcbean Mar 19 '14 at 23:08
  • That would work and is basically what [@ircmaxell suggested on *Preventing Directory Traversal in PHP but allowing paths*](http://stackoverflow.com/a/4205278/53114). – Gumbo Mar 20 '14 at 05:50
-1

I think that the answer to this would be no. The person may still be able to access system files, and the method seems quite worthless. Why not use cURL or something like it to access the page. It's safer but not the safest. But your script won't work in the first place considering,

include_once "sites/$url";

Should be changed to,

include_once "sites/" . $url;

So your script doesn't start sending users to "/sites/$url". You could do this if you want to be secure:

if(isset($_GET['pageid'])) {
    if($_GET['pageid']=='1') {
         include_once 'page1.php';
    }
}

You could make a new $_GET['pageid']== check for every page you need to send the user to.

duper51
  • 770
  • 1
  • 5
  • 20