Register Members List Search Today's Posts Mark Forums Read

Reply
 
Article Options
[How To] Never Worry About SQL Injections and Queries Again
vbresults
Join Date: Apr 2009
Posts: 857

Jackson, WY
by vbresults vbresults is offline 27 Mar 2012

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)

Block Disabled:      (Update License Status)  
Suspended or Unlicensed Members Cannot View Code.

ExtendedDatabases_Query::first (improved way)

Block Disabled:      (Update License Status)  
Suspended or Unlicensed Members Cannot View Code.

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)

Block Disabled:      (Update License Status)  
Suspended or Unlicensed Members Cannot View Code.

ExtendedDatabases_Query::read (improved way)

Block Disabled:      (Update License Status)  
Suspended or Unlicensed Members Cannot View Code.

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)

Block Disabled:      (Update License Status)  
Suspended or Unlicensed Members Cannot View Code.

ExtendedDatabases_Query::field (improved way)

Block Disabled:      (Update License Status)  
Suspended or Unlicensed Members Cannot View Code.


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)

Block Disabled:      (Update License Status)  
Suspended or Unlicensed Members Cannot View Code.

ExtendedDatabases_Query::read with array sub-parameters (improved way)

Block Disabled:      (Update License Status)  
Suspended or Unlicensed Members Cannot View Code.

---

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, 16 views)

Last edited by vbresults : 13 Aug 2012 at 02:09.
Views: 8304
Reply With Quote
Comments
  #2  
Old 28 Mar 2012, 13:51
kh99's Avatar
kh99 kh99 is offline
 
Join Date: Aug 2009
Real name: Kevin
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.
__________________
Please don't PM me - post your questions in the appropriate forum.
Please don't PM me to ask me to read your thread.

Last edited by kh99 : 28 Mar 2012 at 14:02.
Reply With Quote
  #3  
Old 29 Mar 2012, 18:33
vbresults's Avatar
vbresults vbresults is offline
 
Join Date: Apr 2009
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 30 Mar 2012, 00:47
kh99's Avatar
kh99 kh99 is offline
 
Join Date: Aug 2009
Real name: Kevin
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.
__________________
Please don't PM me - post your questions in the appropriate forum.
Please don't PM me to ask me to read your thread.
Reply With Quote
  #5  
Old 30 Mar 2012, 06:25
vbresults's Avatar
vbresults vbresults is offline
 
Join Date: Apr 2009
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 01 May 2012, 18:48
abdobasha2004's Avatar
abdobasha2004 abdobasha2004 is offline
 
Join Date: Aug 2008
your threads are just wonderful man !
__________________
Egypt News website, latest Egyptian news updated instantly.
Reply With Quote
  #7  
Old 01 May 2012, 19:59
vbresults's Avatar
vbresults vbresults is offline
 
Join Date: Apr 2009
Originally Posted by abdobasha2004 View Post
your threads are just wonderful man !
Thank you
Reply With Quote
  #8  
Old 06 May 2012, 14:39
Angel-Wings's Avatar
Angel-Wings Angel-Wings is offline
 
Join Date: Sep 2007
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 22 May 2012, 16:15
vbresults's Avatar
vbresults vbresults is offline
 
Join Date: Apr 2009
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 22 May 2012, 16:51
Andreas's Avatar
Andreas Andreas is offline
 
Join Date: Jan 2004
Real name: Andreas
Prepared Statements (this efectively is smth. "like prepared statements light" ) are great, but hard to extend.

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
  #11  
Old 07 Aug 2012, 03:31
Chris8's Avatar
Chris8 Chris8 is offline
 
Join Date: Nov 2009
Originally Posted by LancerForHireLLC View Post
Download the product-extended_databases_ Xml and add it (extended_databases) as a product dependency. Yes; that's it!
I'm sorry, I' m not sure if I understood it well, the part with adding it as dependency.

To get it running I should upload it and install product-extended_databases-1.2.1.xml you've attached here and then in every mod which I would like to make sure that has strings escaped add it as dependency right into <dependencies>extended_databases</dependencies> tags. And that's it. Do I get it right?
Reply With Quote
  #12  
Old 11 Aug 2012, 17:58
vbresults's Avatar
vbresults vbresults is offline
 
Join Date: Apr 2009
Originally Posted by Chris8 View Post
I'm sorry, I' m not sure if I understood it well, the part with adding it as dependency.

To get it running I should upload it and install product-extended_databases-1.2.1.xml you've attached here and then in every mod which I would like to make sure that has strings escaped add it as dependency right into <dependencies>extended_databases</dependencies> tags. And that's it. Do I get it right?
Put vBulletin in debug mode and add the extended_databases dependency to the product there, preferably with minimum version number 1.4. After you've done this, rewrite the queries using the format above.

---

On another note, 1.4 has been released!
Reply With Quote
  #13  
Old 25 Feb 2013, 17:09
Dorgham's Avatar
Dorgham Dorgham is offline
 
Join Date: May 2012
I'm a big fan of your posts
And learn lots and lots
Thank you
__________________
AdSense revenue sharing 100% - [ Hanan ]
News, egypt : أخبار مصر - Photo, picture : صور
Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
Article Options

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts
Forum Jump


New To Site? Need Help?

All times are GMT. The time now is 11:59.

Layout Options | Width: Wide Color: