vb.org Archive
Page 1 of 2 1 2
Show 40 post(s) from this thread on one page

vb.org Archive (https://vborg.vbsupport.ru/index.php)
-   General Articles (https://vborg.vbsupport.ru/forumdisplay.php?f=189)
-   -   [How To] Never Worry About SQL Injections and Queries Again (https://vborg.vbsupport.ru/showthread.php?t=280738)

vbresults 03-26-2012 10:00 PM
[How To] Never Worry About SQL Injections and Queries Again
 
1 Attachment(s)
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!

kh99 03-28-2012 12:51 PM
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.

vbresults 03-29-2012 05:33 PM
Quote:
Originally Posted by kh99 (Post 2314229)
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? :p

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

My pleasure to share. :)

kh99 03-29-2012 11:47 PM
Quote:
Originally Posted by Lancerforhire (Post 2314777)
...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.

vbresults 03-30-2012 05:25 AM
Quote:
Originally Posted by kh99 (Post 2314869)
I knew it was just an example, I meant that *I* was nit-picking by mentioning it.
Ah, sorry!

abdobasha2004 05-01-2012 05:48 PM
your threads are just wonderful man !

vbresults 05-01-2012 06:59 PM
Quote:
Originally Posted by abdobasha2004 (Post 2325301)
your threads are just wonderful man !
Thank you :)

Angel-Wings 05-06-2012 01:39 PM
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.

vbresults 05-22-2012 03:15 PM
Quote:
Originally Posted by Angel-Wings (Post 2326734)
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.

Andreas 05-22-2012 03:51 PM
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.


All times are GMT. The time now is 02:38 PM.
Page 1 of 2 1 2
Show 40 post(s) from this thread on one page

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.01249 seconds
  • Memory Usage 1,809KB
  • Queries Executed 10 (?)
More Information
Template Usage:
  • (1)ad_footer_end
  • (1)ad_footer_start
  • (1)ad_header_end
  • (1)ad_header_logo
  • (1)ad_navbar_below
  • (8)bbcode_php_printable
  • (6)bbcode_quote_printable
  • (1)footer
  • (1)gobutton
  • (1)header
  • (1)headinclude
  • (6)option
  • (1)pagenav
  • (1)pagenav_curpage
  • (1)pagenav_pagelink
  • (1)post_thanks_navbar_search
  • (1)printthread
  • (10)printthreadbit
  • (1)spacer_close
  • (1)spacer_open 
Phrase Groups Available:
  • global
  • postbit
  • showthread
Included Files:
  • ./printthread.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/class_bbcode_alt.php
  • ./includes/class_bbcode.php
  • ./includes/functions_bigthree.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
  • printthread_start
  • pagenav_page
  • pagenav_complete
  • bbcode_fetch_tags
  • bbcode_create
  • bbcode_parse_start
  • bbcode_parse_complete_precache
  • bbcode_parse_complete
  • printthread_post
  • printthread_complete