is this a security hole?

[trapach]

this looks bad to me, but it solves a lot of problems.

basically I got files inside /app/webroot

I got a .htaccess file, that redirects URLs to /loader.php?include_page=[whatever you type], as follows:

RewriteEngine on
RewriteRule ^(.*)$ loader.php?include=/$1 [QSA,L,NC]

so, for instance, if you visit /search/foo/, apache redirects traffic to /loader.php?include=/search/foo/

eventually (after doing lots of stuff), loader.php includes the page specified in $_GET['include'], in this way:
include '/app/webroot' . $_GET['include']

it looks to me like a bad security hole, but I've tried all possible values I could think of, and nothing bad happened.

I think this doesn't allow loading external sites or system files like /etc/passwd, because the way the include is, the file must be inside /app/webroot, and whatever it's in this directory you access from the web anyway.

am I missing something? is this bad?

any suggestion appreciated.

[logic_earth]

try this value '/../../etc/passwd'
And yes that is a very bad hole.

[trapach]

guess gotta find another way to solve other problems...

thanks!

[UFTimmy]

try this value '/../../etc/passwd'
And yes that is a very bad hole.

This.

[trapach]

... meaning probs the include would solve

[joebert]

What about running any paths containing data from the user through this ?

function path_jailed($path, $jail = '.')
{
    $path = sprintf("$jail/%s", preg_replace('#^[A-Z]+:|\\+|\.+/#i', '/', $path));
    return preg_replace('#/{2,}#', '/', $path);
} 

include path_jailed($_GET['include'], '/app/webroot'); 

[logic_earth]

What about running any paths containing data from the user through this ?

function path_jailed($path, $jail = '.')
{
    $path = sprintf("$jail/%s", preg_replace('#^[A-Z]+:|\\+|\.+/#i', '/', $path));
    return preg_replace('#/{2,}#', '/', $path);
} 

include path_jailed($_GET['include'], '/app/webroot'); 

Or you can just apply "realpath" and get to the same place.

$safe = preg_replace( '~[\\\\/]+~', '/', dirname( __FILE__ ) . '/' );
$user = preg_replace( '~[\\\\/]+~', '/', realpath( $_GET['path'] ) . '/' );

if ( strpos( $user, $safe ) !== 0 ) {
    exit( 'Bad path!' );
} 

Don't even attempt to fix the path, disregard it completely!

[joebert]

That jails you to the application directory regardless though, doesn't it ?
What if you have multiple sites using the same includes ?

[logic_earth]

That jails you to the application directory regardless though, doesn't it ?
What if you have multiple sites using the same includes ?

If the path goes outside of the desired location, or goes to a location that doesn't exists. That would tell me that someone is trying to hack into the system, and/or find an exploit. The common user is not going to forge paths.

And I'm not sure what "...multiple sites using the same includes..." is suppose to mean in this context.

[joebert]

Assume you have a server with the following paths on the filesystem.

/www/domain-one.com/
/www/domain-two.com/
/www/common/

The first path is the DocumentRoot for an Apache virtual host named "domain-one.com", the second path is a DocumentRoot for "domain-two.com", and the third path is a set of common includes accessible by both sites so the administrator doesn't need to maintain two sets of includes for the two sites.

[trapach]

yeah that was a bad idea to start with.

I now got another solution figured out that doesn't affect security.

even though the path jail might work, it it's bad then better not have code like that at all.