Go Back   vb.org Archive > vBulletin Article Depository > Read An Article > General Articles
Reload this Page [How To] Never Worry About SQL Injections and Queries Again
FAQ Community Calendar Today's Posts Search

Reply
Page 1 of 2 1 2
 
Thread Tools
[How To] Never Worry About SQL Injections and Queries Again
vbresults
Join Date: Apr 2009
Posts: 687

 

Show Printable Version Email this Page Subscription
vbresults vbresults is offline 03-26-2012, 10:00 PM
Why use this? Queries are normally cumbersome to write for vBulletin, and when managing a lot of queries in even a small-medium-sized, it is easy to miss sanitizing an input here or there. If you use this tool, the SQL Injection prevention is completely automatic, queries are easier to create and maintain, and blocks of code that are common among similar queries are eliminated.

What is this? This tool prevents SQL injections by placing a layer of protection between the query and database. First, it is important to note that preventing SQL injections is top priority. It takes 1 SQL Injection for someone to take over an entire forum, as history has proven time and time again. After developing a few dozen add-ons, I realized that queries are annoying to write for vB, especially with this fact in mind. I constantly found myself checking queries over and over to make sure everything was safe. It became a big problem for applications that were too small to be moved over to a PHP framework (which has tools like this built in), but too large to police each database query. This tool is intended for add-on authors or people adding custom functionality to their forums themselves.

How is this used? This tool will require only basic knowledge of strings, arrays, functions, and printf's format -- nothing specific to vBulletin. If you are already writing vBulletin queries, then you are mostly ready. How to use this is listed below.

Let's compare the common ways of executing queries versus using Extended Databases.

1. Fetch one record; we are getting the username and userid of a user with the email address we input.

vB_Database::query_first (common way)
PHP Code:
$query = 'SELECT userid, username FROM ' . TABLE_PREFIX . 'user WHERE email = "' . $vbulletin->db->escape_string($vbulletin->GPC['email']) . '"';

$user = $vbulletin->db->query_first($query); 
ExtendedDatabases_Query::first (improved way)
PHP Code:
$user = ExtendedDatabases_Query::first(array(
    'query' => 'SELECT userid, username FROM %1$suser WHERE email = "%2$s"',
    'params' => $vbulletin->GPC['email']
)); 
Both will return exactly the same thing. One will not require direct use of TABLE_PREFIX, vB_Database::escape_string, or bizarre quoting. %1$s will always be the table prefix whether params is an array or not.

2. Fetch multiple records; we are getting the usernames for each user who has a username starting with our input (e.g. we input lancer, it returns lancerforhire).

vB_Database::query_read_slave (common way)
PHP Code:
$query = 'SELECT username FROM ' . TABLE_PREFIX . 'user WHERE username REGEXP "^' . preg_quote($vbulletin->db->escape_string($vbulletin->GPC['username'])) . '"';
$resource = $vbulletin->db->query_read_slave($query);
$records = null;

if (!$vbulletin->db->num_rows($resource)) {
    // no records
}

while ($record = $vbulletin->db->fetch_array($resource)) {
    $records[] = $record;
}

$vbulletin->db->free_result($resource); 
ExtendedDatabases_Query::read (improved way)
PHP Code:
$records = ExtendedDatabases_Query::read(array(
    'query' => 'SELECT username FROM %1$suser WHERE username REGEXP "^%2$s"',
    'params' => preg_quote($vbulletin->GPC['username'])
));

if (!$records) {
    // no records
} 
Once again, both do exactly the same thing. One is just a LOT simpler. SQL injection prevention and other vital functions are AUTOMATIC.

3. Fetch a single field from a single record; we are getting the first full username for the user who has a username starting with our input (e.g. we input lancer, it returns lancerforhire). New in 1.4!

vB_Database::query_first (common way)
PHP Code:
$query = 'SELECT username FROM ' . TABLE_PREFIX . 'user WHERE username REGEXP "^' . preg_quote($vbulletin->db->escape_string($vbulletin->GPC['username'])) . '"';

$user = $vbulletin->db->query_first($query);

$username = $user ? $user['username'] : null; 
ExtendedDatabases_Query::field (improved way)
PHP Code:
$username = ExtendedDatabases_Query::field(array(
    'name' => 'username',
    'query' => 'SELECT username FROM %1$suser WHERE username REGEXP "^%2$s"',
    'params' => preg_quote($vbulletin->GPC['username'])
)); 

4. Fetch multiple records; we are getting the username, user id, and user group id of all users whose primary groups match our input (array). New in 1.4!

vB_Database::query_read_slave (common way)
PHP Code:
$query = 'SELECT username, userid, usergroupid FROM ' . TABLE_PREFIX . 'user
    WHERE usergroup IN (' . explode(', ', $vbulletin->GPC['usergroupids']) . ')';

$resource = $vbulletin->db->query_read_slave($query);
$records = null;

if (!$vbulletin->db->num_rows($resource)) {
    // no records
}

while ($record = $vbulletin->db->fetch_array($resource)) {
    $records[] = $record;
}

$vbulletin->db->free_result($resource); 
ExtendedDatabases_Query::read with array sub-parameters (improved way)
PHP Code:
$records = ExtendedDatabases_Query::read(array(
    'query' => 'SELECT username, userid, usergroupid FROM %1$suser
        WHERE %2$s'
    'params' => array(
        'usergroupid' => $vbulletin->GPC['usergroupids']
    )
));

if (!$records) {
    // no records
} 
---

ExtendedDatabases_Query::write is the successor to vB_Database::query_write; I do not believe this warrants another example.

---

Implement this Right Now

If you are a coder, implement this immediately. If you are not a coder, ask the developers of your installed add-ons to do it. It's very simple and has long term benefits, but if you don't do it right now, chances are you never will (unless your forum implodes).

---

How do I get this?

Download the product-extended_databases_ Xml and add it (extended_databases) as a product dependency. Yes; that's it!
Attached Files
File Type: xml product-extended_databases-1.4.xml (4.2 KB, 23 views)
Reply With Quote
  #2  
Old 03-28-2012, 12:51 PM
kh99 kh99 is offline
 
Join Date: Aug 2009
Location: Maine
Posts: 13,185
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
Interesting idea - it definitely would help by making sure that any strings are escaped. However, I don't agree with your assertion that it's "a lot simpler". In fact, I find the second example to be a bit misleading because normally you would not collect all the records in an array, but instead you'd process them in a loop. So the second case using your product would require a for loop after it to process the records - the same loop that appears in the "common way" example. Also, collecting all the records in an array holds them all in memory at the same time, which could be an issue for a query that returns a lot of large records. One more thing that I admit is nit-picking - you probably would not need to worry about escaping a userid that was passed in $vbulletin->GPC['userid'] because you would have "cleaned" it using TYPE_INT (and you'd probably want to check for it being a postivie integer before going ahead with a query).

That said, it's a good idea, and there are probably people who aren't confident in their ability to ensure that all strings are escaped before using them in a query. Can't argue with the fact that that's been the issue in a couple of recent mod security problems, so using this would probably have avoided them.

Thanks for sharing this.
Reply With Quote
  #3  
Old 03-29-2012, 05:33 PM
vbresults vbresults is offline
 
Join Date: Apr 2009
Posts: 687
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
Quote:
Originally Posted by kh99 View Post
Interesting idea - it definitely would help by making sure that any strings are escaped. However, I don't agree with your assertion that it's "a lot simpler". In fact, I find the second example to be a bit misleading because normally you would not collect all the records in an array, but instead you'd process them in a loop. So the second case using your product would require a for loop after it to process the records - the same loop that appears in the "common way" example. Also, collecting all the records in an array holds them all in memory at the same time, which could be an issue for a query that returns a lot of large records. One more thing that I admit is nit-picking - you probably would not need to worry about escaping a userid that was passed in $vbulletin->GPC['userid'] because you would have "cleaned" it using TYPE_INT (and you'd probably want to check for it being a postivie integer before going ahead with a query).

That said, it's a good idea, and there are probably people who aren't confident in their ability to ensure that all strings are escaped before using them in a query. Can't argue with the fact that that's been the issue in a couple of recent mod security problems, so using this would probably have avoided them.

Thanks for sharing this.
It would make sure that any strings are escaped as you said. Most add-on queries that would cause a performance hit can have the limit adjusted or results paginated. Also, note that the read example was just that -- an example. I'll adjust it, but who's nit-picking now?

I really do hope coders start using this; it would change hundreds if not thousands of lives.

My pleasure to share.
Reply With Quote
  #4  
Old 03-29-2012, 11:47 PM
kh99 kh99 is offline
 
Join Date: Aug 2009
Location: Maine
Posts: 13,185
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
Quote:
Originally Posted by Lancerforhire View Post
...Also, note that the read example was just that -- an example. I'll adjust it, but who's nit-picking now?
I knew it was just an example, I meant that *I* was nit-picking by mentioning it.
Reply With Quote
Благодарность от:
vbresults
  #5  
Old 03-30-2012, 05:25 AM
vbresults vbresults is offline
 
Join Date: Apr 2009
Posts: 687
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
Quote:
Originally Posted by kh99 View Post
I knew it was just an example, I meant that *I* was nit-picking by mentioning it.
Ah, sorry!
Reply With Quote
  #6  
Old 05-01-2012, 05:48 PM
abdobasha2004's Avatar
abdobasha2004 abdobasha2004 is offline
 
Join Date: Aug 2008
Posts: 541
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
your threads are just wonderful man !
Reply With Quote
  #7  
Old 05-01-2012, 06:59 PM
vbresults vbresults is offline
 
Join Date: Apr 2009
Posts: 687
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
Quote:
Originally Posted by abdobasha2004 View Post
your threads are just wonderful man !
Thank you
Reply With Quote
  #8  
Old 05-06-2012, 01:39 PM
Angel-Wings's Avatar
Angel-Wings Angel-Wings is offline
 
Join Date: Sep 2007
Posts: 206
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
Hmm - wouldn't a good way be to use stored procedures ?

Then this problem is solved on the DB level instead of trying to escape some input - which won't hurt too for sure.

And the DB user would just need the EXECUTE permission, no more "dangerous" things like CREATE TABLE, DROP etc.
Reply With Quote
  #9  
Old 05-22-2012, 03:15 PM
vbresults vbresults is offline
 
Join Date: Apr 2009
Posts: 687
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
Quote:
Originally Posted by Angel-Wings View Post
Hmm - wouldn't a good way be to use stored procedures ?

Then this problem is solved on the DB level instead of trying to escape some input - which won't hurt too for sure.

And the DB user would just need the EXECUTE permission, no more "dangerous" things like CREATE TABLE, DROP etc.
IIRC you'd need to create another connection to the DB and it's more complex. I could be wrong on the connection part, but either way it's far more complex.
Reply With Quote
  #10  
Old 05-22-2012, 03:51 PM
Andreas's Avatar
Andreas Andreas is offline
 
Join Date: Jan 2004
Location: Germany
Posts: 6,863
Благодарил(а): 0 раз(а)
Поблагодарили: 0 раз(а) в 0 сообщениях
Default
Prepared Statements (this efectively is smth. "like prepared statements light" ) are great, but hard to extend.

Quote:
Hmm - wouldn't a good way be to use stored procedures ?
*eek*
You would need a stored precedure for everything but the kitchen sink to really prevent all prossible injections.
Reply With Quote
Reply
Page 1 of 2 1 2

« Previous Thread | Next Thread »

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts
BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Forum Jump


All times are GMT. The time now is 09:37 AM.

Contact Us - Archive - Top

Powered by vBulletin® Version 3.8.12 by vBS
Copyright ©2000 - 2024, vBulletin Solutions Inc.
X vBulletin 3.8.12 by vBS Debug Information
  • Page Generation 0.05104 seconds
  • Memory Usage 2,361KB
  • Queries Executed 24 (?)
More Information
Template Usage:
  • (1)SHOWTHREAD
  • (1)ad_footer_end
  • (1)ad_footer_start
  • (1)ad_header_end
  • (1)ad_header_logo
  • (1)ad_navbar_below
  • (1)ad_showthread_beforeqr
  • (8)bbcode_php
  • (6)bbcode_quote
  • (1)footer
  • (1)forumjump
  • (1)forumrules
  • (1)gobutton
  • (1)header
  • (1)headinclude
  • (1)modsystem_article
  • (1)navbar
  • (4)navbar_link
  • (120)option
  • (1)pagenav
  • (1)pagenav_curpage
  • (1)pagenav_pagelink
  • (10)post_thanks_box
  • (2)post_thanks_box_bit
  • (10)post_thanks_button
  • (1)post_thanks_javascript
  • (1)post_thanks_navbar_search
  • (2)post_thanks_postbit
  • (10)post_thanks_postbit_info
  • (9)postbit
  • (1)postbit_attachment
  • (10)postbit_onlinestatus
  • (10)postbit_wrapper
  • (1)spacer_close
  • (1)spacer_open
  • (1)tagbit_wrapper 
Phrase Groups Available:
  • global
  • inlinemod
  • postbit
  • posting
  • reputationlevel
  • showthread
Included Files:
  • ./showthread.php
  • ./global.php
  • ./includes/init.php
  • ./includes/class_core.php
  • ./includes/config.php
  • ./includes/functions.php
  • ./includes/class_hook.php
  • ./includes/modsystem_functions.php
  • ./includes/functions_bigthree.php
  • ./includes/class_postbit.php
  • ./includes/class_bbcode.php
  • ./includes/functions_reputation.php
  • ./includes/functions_post_thanks.php 
Hooks Called:
  • init_startup
  • init_startup_session_setup_start
  • init_startup_session_setup_complete
  • cache_permissions
  • fetch_threadinfo_query
  • fetch_threadinfo
  • fetch_foruminfo
  • style_fetch
  • cache_templates
  • global_start
  • parse_templates
  • global_setup_complete
  • showthread_start
  • showthread_getinfo
  • forumjump
  • showthread_post_start
  • showthread_query_postids
  • showthread_query
  • bbcode_fetch_tags
  • bbcode_create
  • showthread_postbit_create
  • postbit_factory
  • postbit_display_start
  • post_thanks_function_post_thanks_off_start
  • post_thanks_function_post_thanks_off_end
  • post_thanks_function_fetch_thanks_start
  • fetch_musername
  • post_thanks_function_fetch_thanks_end
  • post_thanks_function_thanked_already_start
  • post_thanks_function_thanked_already_end
  • post_thanks_function_fetch_thanks_bit_start
  • post_thanks_function_show_thanks_date_start
  • post_thanks_function_show_thanks_date_end
  • post_thanks_function_fetch_thanks_bit_end
  • post_thanks_function_fetch_post_thanks_template_start
  • post_thanks_function_fetch_post_thanks_template_end
  • postbit_imicons
  • bbcode_parse_start
  • bbcode_parse_complete_precache
  • bbcode_parse_complete
  • postbit_attachment
  • postbit_display_complete
  • post_thanks_function_can_thank_this_post_start
  • pagenav_page
  • pagenav_complete
  • tag_fetchbit_complete
  • forumrules
  • navbits
  • navbits_complete
  • showthread_complete