Securing against SQL Injection?

[|Jordan|]

What do i look for in my modifications to see if they are susceptible to SQL Injection? What needs to be changed?

P.S. I also made this thread in VB3 forum because i have 2 forums, 1 runs VB4 and the other runs VB3.

[Disasterpiece]

Don't use vars in query-context which can be altered by the user in any way without sanitizing.
This includes $_GET, $_POST, $_COOKIE vars, as well as data which can be loaded from the database.

run $vbulletin->db->escape_string($myVar) on anything and you can be pretty sure that this won't be injected.

//e:

btw, it's a php-related issue and has not really anything to do with vbulletin or the vbulletin version.

[Sarteck]

One thing I do (though it doesn't really "protect" anything, it can help) is use the sprintf() function when building my SQL queries.

For example, if you have a PHP line line this:

$query = "SELECT * FROM user WHERE userid=$userid";

Then, depending on what goes into $userid you might possibly be vulnerable. However, if you re-write the query like this, instead:

$query = sprintf("SELECT * FROM user WHERE userid=%d",$userid);

Then the %d can only be replaced by a number.

As Disasterpiece mentioned, make sure to sanitize all variables that might be used in queries. In fact, it's a good idea to sanitize EVERYTHING except for the ['do'] variables, for the most part.

Use $MyVar = $vbulletin->input->clean_gpc('p', 'MyVar', TYPE_INT); instead of $MyVar = $_POST['MyVar'];, for example.

[|Jordan|]

What about the tips suggested here (http://www.tizag.com/mysqlTutorial/mysql-php-sql-injection.php), most notably mysql_real_escape_string.

[Disasterpiece]

Yeah, that's what the $vbulletin->db->escape_string() function is for, all along with multiple other functions which sanitize strings for queries.

That's enough for a modder to know if he wants to make his/her modifications safe.
But sadly, as the latest events show, some "coders" aren't even capable of implementing the simplest security routines.

Anyway, I'm not sure where you want to go with your thread. We already know how to prevent SQL queries, modders (should) know and you seem to be informed there as well.

[|Jordan|]

I'm trying to be proactive and sanitize the mods myself instead of waiting for either my forum or someone else's to get hacked, wait for a patch or disable the modification.

[Disasterpiece]

Right.

So the biggest security risks are: SQL Queries, eval() functions and anything which has to do with file operations like fopen(), fread(), file_get_contents(), include(), require(), etc.

When you look through the mods, look out for these areas and check what vars they are using. Then try to track them back where they came from and what has been done with them since they have been defined.
The easiest thing to spot is if they come directly from user-input vars like $_GET, $_POST, $_FILE, $_COOKIE or $_REQUEST, but also from $_SERVER and $_ENV which is NOT safe, altough you might think it. Even HTTP-Referrer and User-Agents can be faked, as well as cookie values.

However, you track these variables back, until you find some functionality which sanitizes them.
For integers, it's quite easy to do, like
$foo = (int)$var;
$foo = intval($var);
$foo = sprintf("%d", $var);
$foo = 0+$var;
etc.
A good but very advanced method of sanitizing strings is regex. It's mostly safe and a good idea to use it, given that you have enough knowledge to actually use it right.
$db->escape_string() works for most strings, it just escapes anything which could mess up the query like quotes, comment-chars, nullbytes, etc.

If you get to the point, where vbulletin functions are used to sanitize gpc vars, you might want to check the includes/class.core.php file, find the vB_gpc class and see what the "clean" vars do, because like the past showed us, not everyone knows what they actually do. Not all of them "clean" a string for usage in queries or evals, they just make according typecast which has nothing to do with security.

If you are unsure weather a mod has a security flaw or not, ask in the forums, PM a staff member or PM me if you like.

[|Jordan|]

You are a machine!

Thanks for all these tips. If i come up with anything suspicious, ill let everyone know.