xss/sql injection?

[karimali831]

I'm confused, can this result in css/sql injection?

 

if(isset($_GET['action'])){
if($_GET['action'] == 'details'){
	$cupID = $_GET['cupID'];
	$ergebnis = safe_query("SELECT gameaccID, name, start, ende, typ, game, `desc`, status, checkin, maxclan, gewinn1, gewinn2, gewinn3 FROM ".PREFIX."cups WHERE ID = '".$cupID."'");
	$ds=mysql_fetch_array($ergebnis);

...

 

Some german fellow was explaining, translate to English briefly:

 

"$ CupID is not escaped. NEN here I could just "; DROP TABLE` cups `Paste and your table is no longer available eez. Or I could inject javascript, your current session read out, accept it and act as an admin ... "

 

I am trying to understand what he means by this... is this query vulnerable to an injection and why/how? 

[analog]

$cupID is coming straight from the query string so if someone changed the query string value to "?cupID=0'; DROP TABLE cups" you would end up with a query like this:

 

SELECT
gameaccID, name, start, ende, typ, game, `desc`, status, checkin, maxclan, gewinn1, gewinn2, gewinn3
FROM
cups
WHERE
ID = '0';

DROP TABLE
cups // goodbye table cups

 

To stop that happening you should use mysql_real_escape_string to escape all user input. So:

 

$cupID = mysql_real_escape_string($_GET['cupID']);

 

There is a better explanation here: http://www.tizag.com/mysqlTutorial/mysql-php-sql-injection.php

[codebyren]

It depends on what your safe_query() function does?  If it intelligently 'escapes' the $cupID then you should be ok from an SQL injection perspective.

[Zurev]

You want to stick by the FIEO concept, filter input, escape output.

 

Only allow input into your database fields that are applicable, use everything you need, regular expression matching is your friend when accepting input.

 

Escaping output? At bare minimum, anything coming from a database should be filtered like so:

$output = htmlentities($output, ENT_QUOTES, "UTF-8"); // Or applicable charset

What this does is converts any applicable characters that can be translated into html into it's html, so <'s and other characters aren't considered part of the source code of the website.

 

Other things to look into:

http://php.net/manual/en/security.magicquotes.php (Applicable you're running a version < PHP 5.3.0)

http://www.php.net/manual/en/security.database.sql-injection.php

[chaseman]

I'm just wondering, if the OP would have went with this un-escaped version, is there any way for somebody to tell that it's actually un-escaped and there's a hole, or do people do these hacks with trial and error?

[Zurev]

I'm just wondering, if the OP would have went with this un-escaped version, is there any way for somebody to tell that it's actually un-escaped and there's a hole, or do people do these hacks with trial and error?

 

Trial and error is a large part of it, though there are applications out there and even browser extensions/plugins that can check for flaws. (See XSS me for XSS checking)

An open phpinfo file is an attackers dream, though they are rare to be left out in the open. Not using error suppression properly can let a user see full root paths, the database name or table name (very bad because they would even know what table to drop), also there are a ton of tests out there you can use to see which version of MySQL a server is running which can then help the attacker know exactly what the website could be vulnerable to.

 

Amazing resource for security in web applications:

http://owasp.org

[karimali831]

This is the safe_query() function:

 

$_mysql_querys = array();
function safe_query($query="") {
global $_mysql_querys;
if(stristr(str_replace(' ', '', $query), "unionselect")===FALSE AND stristr(str_replace(' ', '', $query), "union(select")===FALSE){
	$_mysql_querys[] = $query;
	if(empty($query)) return false;
	if(DEBUG == "OFF") $result = mysql_query($query) or die('Query failed!');
	else {
		$result = mysql_query($query) or die('Query failed: '
		.'<li>errorno='.mysql_errno()
		.'<li>error='.mysql_error()
		.'<li>query='.$query);
	}
	return $result;
}
else die();
}

 

Does it escape $cupID?

If not a lot of work needs to be done as I have coded many things like this.

 

Thanks

 

[codebyren]

Does it escape $cupID?

If not a lot of work needs to be done as I have coded many things like this.

 

No, you'll need to have a look at what some of the other users suggested in response to your original question.