Register Members List Search Today's Posts Mark Forums Read

Reply
 
Thread Tools
  #1  
Old 28 Feb 2012, 22:07
rpgamersnet's Avatar
rpgamersnet rpgamersnet is offline
 
Join Date: Jul 2004
Lightbulb Alternate fix to injection code in comments

So, in another thread it was mentioned that the current fix may get the job done, its also filtering out good data. There must be some proper solution to handle incoming comment data securely. I thought I would start a discussion in regards to finding an alternate fix to the problem then the one currently available.

The problem: Users input data into comments that is executed and causes trouble.

Solution ideas: Escape incoming data so that it cannot execute? Allow only alphanumeric comment data and write the SQL statements so that they cannot be broken out?

I will be the first to admit I am not a professional coder, although I do write a lot of code myself. I haven't taken a long look at how the comments are currently handled, but plan to. Lets pool some ideas and help MrZeroPage come up with a more solid fix!
Reply With Quote
Comments
  #2  
Old 29 Feb 2012, 07:15
Sarteck's Avatar
Sarteck Sarteck is offline
 
Join Date: Mar 2008
I do a few things.


First off, I almost ALWAYS use sprintf(). It's pretty awesome.


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

Bam, you'll always get an integer. Also, query looks hella prettier. :3

Two, why not use vBulletin's built-in cleaning functions on data? That would solve a lot of it, wouldn't it?

Mind you, I'm a complete newbie to the scripting of this modification in particular, but I have successfully programmed a bunch of homebrewed mods for my own. I just want a disclaimer here that I could be completely off-base. X3
Reply With Quote
  #3  
Old 29 Feb 2012, 08:00
stangger5's Avatar
stangger5 stangger5 is offline
 
Join Date: Jan 2005
Starting with 2.7.1+

To fix that exploit was to edit one line..


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

change to

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

Originally Posted by BirdOPrey5
Comment should be OK because of they way strings are put in the database. The problem was s_id was allowed to be a string when it was supposed to be an int, that is what allowed the exploit.

The ibp_cleansql function needs to be changed to accept a second argument that says what type of data it is (string or int) and clean it differently depending on what it is supposed to be.

vBulletin has built in cleaning functions too that can/should be used.
__________________
vb ibProArcade: Download Game, Game Challenge, Report Game Systems.
Need Custom Arcade Work ?? ,,Plus more can be found at: next-level-arcade.com
Reply With Quote
  #4  
Old 29 Feb 2012, 09:13
Sarteck's Avatar
Sarteck Sarteck is offline
 
Join Date: Mar 2008
See? Perfect example of where sprintf() would be put to awesome use. Just use %d in your query and you're good to go.
Reply With Quote
  #5  
Old 29 Feb 2012, 14:24
rpgamersnet's Avatar
rpgamersnet rpgamersnet is offline
 
Join Date: Jul 2004
Wait, so it wasn't the comments that were causing the problem, but the S_ID? My board personally was not hit by this exploit, so I did not have the details.
Reply With Quote
  #6  
Old 29 Feb 2012, 19:11
Mark.B Mark.B is offline
 
Join Date: Feb 2004
Originally Posted by rpgamersnet View Post
Wait, so it wasn't the comments that were causing the problem, but the S_ID? My board personally was not hit by this exploit, so I did not have the details.
I have a feeling that the code Stangger has posted was the fix for the exploit that was fixed in 2.7.1, rather than 2.7.2.
Reply With Quote
  #7  
Old 29 Feb 2012, 20:18
stangger5's Avatar
stangger5 stangger5 is offline
 
Join Date: Jan 2005
Originally Posted by Mark.B View Post
I have a feeling that the code Stangger has posted was the fix for the exploit that was fixed in 2.7.1, rather than 2.7.2.
I didnt know anything about a exploit with 2.7.2..
__________________
vb ibProArcade: Download Game, Game Challenge, Report Game Systems.
Need Custom Arcade Work ?? ,,Plus more can be found at: next-level-arcade.com
Reply With Quote
  #8  
Old 29 Feb 2012, 20:36
Mark.B Mark.B is offline
 
Join Date: Feb 2004
Originally Posted by stangger5 View Post
I didnt know anything about a exploit with 2.7.2..
No I meant FIXED in 2.7.2.
Reply With Quote
  #9  
Old 29 Feb 2012, 20:47
Hippy's Avatar
Hippy Hippy is offline
 
Join Date: Dec 2001
the only thing needed is what stangger posted above..

--------------- Added 29 Feb 2012 at 20:48 ---------------

Originally Posted by Mark.B View Post
I have a feeling that the code Stangger has posted was the fix for the exploit that was fixed in 2.7.1, rather than 2.7.2.
I didnt know anything about a exploit with 2.7.2.. either

Last edited by Hippy : 29 Feb 2012 at 20:55.
Reply With Quote
  #10  
Old 29 Feb 2012, 20:51
stangger5's Avatar
stangger5 stangger5 is offline
 
Join Date: Jan 2005
Originally Posted by Mark.B View Post
No I meant FIXED in 2.7.2.
Had me going ...lol...
__________________
vb ibProArcade: Download Game, Game Challenge, Report Game Systems.
Need Custom Arcade Work ?? ,,Plus more can be found at: next-level-arcade.com
Reply With Quote
  #11  
Old 29 Feb 2012, 20:58
Mark.B Mark.B is offline
 
Join Date: Feb 2004
But the code Stangger has posted is NOT what changed in 2.7.2.
Reply With Quote
  #12  
Old 29 Feb 2012, 21:12
Hippy's Avatar
Hippy Hippy is offline
 
Join Date: Dec 2001
stangger5 knows this mod better than anyone here.. so trust what he says ...
he has tested this out for the last few days...
Reply With Quote
  #13  
Old 29 Feb 2012, 22:19
stangger5's Avatar
stangger5 stangger5 is offline
 
Join Date: Jan 2005
Originally Posted by Mark.B View Post
But the code Stangger has posted is NOT what changed in 2.7.2.
MrZ changed this:
2.7.1

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

to this:
2.7.2

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

I have this:


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

MrZ`s code is tring to clean the int data .
I`m no guru like MrZ...

--------------- Added 29 Feb 2012 at 22:29 ---------------

To get this thread back on track,,here is a very good read for the ones wanting to learn some of the vBulletin Input Cleaner..

Using the vBulletin Input Cleaner
__________________
vb ibProArcade: Download Game, Game Challenge, Report Game Systems.
Need Custom Arcade Work ?? ,,Plus more can be found at: next-level-arcade.com

Last edited by stangger5 : 29 Feb 2012 at 22:29.
Reply With Quote
  #14  
Old 29 Feb 2012, 22:35
rpgamersnet's Avatar
rpgamersnet rpgamersnet is offline
 
Join Date: Jul 2004
I guess my question was just if the other part that was added is needed, the looping replace function that removes SQL words from comments (but also removes good data). It is near the bottom of the 2.7.2 arcade.php ... needed or just playing it safe?
Reply With Quote
  #15  
Old 29 Feb 2012, 22:47
stangger5's Avatar
stangger5 stangger5 is offline
 
Join Date: Jan 2005
I think,,its playing it safe...Which is not a bad thing these days...

I`m looking into using the vBulletin Input Cleaner instead of the ibp_cleansql..
__________________
vb ibProArcade: Download Game, Game Challenge, Report Game Systems.
Need Custom Arcade Work ?? ,,Plus more can be found at: next-level-arcade.com
Reply With Quote
Reply


Similar Threads
Thread Thread Starter Forum Replies Last Post
Fix overlapping blog comments images shahz vBulletin 4.x Template Modifications 2 04 Nov 2010 03:17
Image with Alternate Text BB Code mikkitine vBulletin 3.7 Add-ons 7 02 Jul 2008 01:29

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

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 06:32.

Layout Options | Width: Wide Color: