Page 1 of 1

SQL Injection Question

Posted: Thu Jun 21, 2007 7:50 pm
by SidewinderX
Well I have this piece of code which grabs some data from the database. However I managed to figure out how to use an SQL Injection to display all the users password hashes which is no good...

Lets assume the file is called members.php. By accessing members.php?n=<sql injection here> it will inject the sql query and display the hashes.

$n is a variable defined in config.php and this script does not use any GET variables where $n can be altered. $n is simply a number denoting how many installs of the software are in the same database, it is essentially used to define the proper table prefix as you can see in the code.

My main question is WHY does this work. Also any advise on how to fix it is also appreciated.

Code: Select all

<?php

$c_xroot  = "./../";
include ($c_xroot . "modname.php");

$onlinetime = 10;
$include    = 0;


if( !defined ("c_COMMON_INCLUDED") ) {
	$c_root = $c_xroot;
	require_once ( $c_root . "includes/functions_common.php" );
}

if ( $include == 1 ) {
	include_once ( $c_root . "_header.php" );
}

if(!defined("c_LAST_ACTIVITY") && $whoisonline == 1 && !empty($GPC['cid'])) {
	define("c_LAST_ACTIVITY", 1);
	$cdb->query("UPDATE c".$n."_member SET lastactivity = '".time()."' WHERE memberid = '".$GPC['cid']."'");
}
?>
<table border="0" cellpadding="0" cellspacing="0" align="center">
	<tr>
		<td align="center" colspan="2"><b>Activities in the last <?php echo $onlinetime; ?> minutes</b></td>
	</tr>
<?php
$query = $cdb->query("
	SELECT memberid, name, lastactivity
	FROM c".$n."_member WHERE lastactivity > ".(time() - $onlinetime * 60)."
");
if ($cdb->num_rows($query) == 0)
{
?>
	<tr>
		<td align="center" width="100%">Nobody online right now</td>
	</tr>
<?php
}
else {
	while ($row = $cdb->fetch_array($query)) {
		dbSelect ($row);
	?>
	<tr>
		<td align="left" width="70%">&raquo; <a href="modules.php?name=<?php echo $cmod; ?>&file=member&action=profile&memberid=<?php echo $row['memberid']; ?>"><?php echo $row['name']; ?></a></td>
		<td align="right" width="30%"><?php echo date("H:i",$row['lastactivity']); ?></td>
	</tr>
	<?php
	}
}
$cdb->free_result($query);
?>
</table>
<?php
if ( $include == 1 ) {
	include_once ( $c_root . "_footer.php" );
}
?>
Just as a side note, this file does not include config.php directly. Rather it includes functions_common.php which includes config.php which contains the value of $n

Posted: Thu Jun 21, 2007 7:57 pm
by feyd
$include is zero, therefore config.php will not be loaded. Your server appears to have register_globals on thereby letting anyone inject variables into your code that may not be set at their use.

Posted: Thu Jun 21, 2007 8:13 pm
by SidewinderX
feyd wrote:$include is zero, therefore config.php will not be loaded. Your server appears to have register_globals on thereby letting anyone inject variables into your code that may not be set at their use.
I apologize, config.php is loaded in the functions_common.php file not _header.php, however in config.php

Code: Select all

$n = "";
Does that mean the variable is not set, or it is set - but set to null? I am assuming the former.

Posted: Thu Jun 21, 2007 8:23 pm
by feyd
What does modname.php define or include?

Also, the include of _header.php references $c_root, which will only be set if c_COMMON_INCLUDED was not defined.

Posted: Thu Jun 21, 2007 9:24 pm
by SidewinderX
feyd wrote:What does modname.php define or include?

Also, the include of _header.php references $c_root, which will only be set if c_COMMON_INCLUDED was not defined.
modname.php defines $cmod

Code: Select all

<?php
$cmod = basename(dirname(__FILE__));
?>
And thanks for pointing out that other error, I never include the header or footer so that functionality was never tested. :oops:

Posted: Thu Jun 21, 2007 9:34 pm
by feyd
So we're sure that c_COMMON_INCLUDED is not defined where it's tested, have you traced the execution inside functions_common.php?

Posted: Thu Jun 21, 2007 10:02 pm
by SidewinderX
feyd wrote:So we're sure that c_COMMON_INCLUDED is not defined where it's tested, have you traced the execution inside functions_common.php?
Yes, up to that point c_COMMON_INCLUDED is not defined. functions_common.php later defines it as 1. What execution was a tracing?

Posted: Thu Jun 21, 2007 10:07 pm
by feyd
Traced the execution to make sure config.php you think should be loaded is indeed being loaded.

Posted: Thu Jun 21, 2007 10:16 pm
by SidewinderX
Yes - the 11th line of functions_common.php loads config.php

Code: Select all

define ("c_COMMON_INCLUDED", 1);
define ("c_RELEASE", 14);
if ( !isset ($startrendertime) ){
	$startrendertime = microtime();
}
error_reporting(E_ALL & ~E_NOTICE);
set_magic_quotes_runtime(0);
$devmode = 0;
$phpversion_array = phpversion();
$phpversion_nr    = $phpversion_array[0] . "." . $phpversion_array[2] . "." . $phpversion_array[4];
include($c_root . "includes/config.php");

Posted: Thu Jun 21, 2007 10:36 pm
by feyd
It's generally not a good idea to turn off E_NOTICE errors.

If you're sure it's loading the correct config.php, then you need to trace its execution. This is no longer a security issue, I'm going to move it to PHP-Code.

Posted: Thu Jun 21, 2007 10:49 pm
by Benjamin
Was the SQL Injection issue ever solved? :?

Posted: Thu Jun 21, 2007 10:59 pm
by SidewinderX
astions wrote:Was the SQL Injection issue ever solved? :?
I believe the issue is that my registered globals are on and $n is not defined.

Posted: Thu Jun 21, 2007 11:13 pm
by Benjamin
So your security issue was solved then. :roll:

Posted: Thu Jun 21, 2007 11:21 pm
by feyd
The security issue is a side-effect, not the actual problem.