nicholasmr / obblm

Automatically exported from code.google.com/p/obblm
26 stars 54 forks source link

MySQL injection #127

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Any concern with MySQL injection?  After speaking with someone they 
noticed there may not be any check for the $input array.

Though mysql_query doesn't allow multiple queries, it may allow UNION to 
coninue the statement and allow a user to get information from the 
database.  I know the front end of the match entry is checked, but someone 
could write their own form and submit it with injections.

I will also be writing checks for my parsing and uploading of the XML 
report to ensure the data is what is expected.

Understanding that maybe only logged in coaches could exploit it this way, 
I would still be concerned.

Original issue reported on code.google.com by funnyfin...@hotmail.com on 14 Jun 2009 at 11:08

GoogleCodeExporter commented 9 years ago
Be more specific.

Where in the code? $input is used many places.

Users are not able to decide the input to the SQL handling methods.
Even if this was possible the SQL-writing methods are protected by users having 
to be
logged in to use them.

Original comment by Nimda...@gmail.com on 15 Jun 2009 at 12:13

GoogleCodeExporter commented 9 years ago
If you're escaping input from forms, you're safe enough for this kind of 
application.
Like mysql_escape_string() or similar. I think I've seen stuff like that in 
OBBLM
code but might be missing somewhere?

The correct way is to also clean/wash input for non-wanted stuff. (Ie, a field 
that
should only contain numbers is cleaned of anything not a number...and truncated 
to
correct length if trying some overflowing trick. Strings get a bit more 
complicated
to clean.)

To hack OBBLM you'd like to print contents of settings.php to the screen - to 
get
access to DB user/pwd. Access to filesystem/shell would be better (from hacker 
point
of view), but harder to accomplish.

Considering how many installs of OBBLM there are out there - this might be
overkill...until someone hacks it. :-P Really it depends on how high the 
ambition is
to make OBBLM hack safe. It's not like we're handling real money in OBBLM.

I would strongly advice AGAINST installing OBBLM in the same DB as a webshop, or
similar. But that's just common sense...

Just my 5 cents.

/Daniel

Original comment by blodae@gmail.com on 15 Jun 2009 at 8:58

GoogleCodeExporter commented 9 years ago
Agreed.

Yes, mysql_escape_*() is used for all strings.

Original comment by Nimda...@gmail.com on 15 Jun 2009 at 10:19

GoogleCodeExporter commented 9 years ago
SQLinjection is usually that you write SQL statements in form field(s) to 
execute SQL
code in the DB (like: select user, password from users). If all SQL's are 
escaped you
cant do such a thing.

If you're trying to inject SQL in a form or overflowing a field you'll most 
likely
just get an error when you have escaped the input - no harm done, and no 
sensitive
information revealed.

Ie, as long as all SQL's are escaped it's good enough security for OBBLM IMHO.

/Daniel

Original comment by blodae@gmail.com on 15 Jun 2009 at 12:32

GoogleCodeExporter commented 9 years ago
The $input referred to is in the match create and entry methods.

I will see if I can come up with a "hack" maybe tonight.

All that would have to be done is to is to send each piece of the array that is 
to 
be an integer to a function that will return it in same condition if it is an 
integer and return it as a 0 if it is something else and process it as a 0.

For text I do not know for certain if the escapes are good enough, they would 
just 
keep it from looking like a malformed statement.  You could add in code in 
there 
still.

Example for inupt of entry:  Make interception = , 0, 0, 0, 0, 0) put statement 
here -- to comment out rest of the line.  The statement would have to work with 
the 
first part but may be able to work with a UNION.  I really don't know MySQL 
much, 
but again the check should be easy enough to avoid any such problem.

Original comment by funnyfin...@hotmail.com on 15 Jun 2009 at 5:07

GoogleCodeExporter commented 9 years ago
Yeah of course, but really mate, focus your attention on other stuff, this is 
not a
serious thing/worth your time. You have to weigh this security threat against 
by who
and for what this application is used for.

Original comment by Nimda...@gmail.com on 15 Jun 2009 at 8:17

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Strange I don't see comment #7 here.

Was gonna say touche on the semi colon but mysql_query doesn't allow multiple 
statements directly.  It looks like you are correct about the no unescaped 
characters, but the integers are not escape and are not checked to be integers. 
 I 
have started work on my report checking though, as it is much easier to hack 
and add 
stuff inside of the XML than it is to create a form to POST and login.

Original comment by funnyfin...@hotmail.com on 16 Jun 2009 at 1:39

GoogleCodeExporter commented 9 years ago
http://test.stuntyleeg.com/leegmgr/botocsreport.xsd

Here is my XSD to help avoid the incorrect data.

Is there a specific pattern that strings should follow?  Like a-z A-Z 0-9 and 
such?  
I will use that to restrict the team and player names in the report.

Original comment by funnyfin...@hotmail.com on 22 Jun 2009 at 2:14

GoogleCodeExporter commented 9 years ago
In general string associated with players, teams, coach, match, tours and the 
other
object types do not have any restrictions on them.

Original comment by Nimda...@gmail.com on 22 Jun 2009 at 3:01

GoogleCodeExporter commented 9 years ago
I think you may strip slashes or something.

Like would you allow:

Th|s i$ /\/\Y Team" Name

Maybe instead of characters you allow, you can give me characters not allowed 
or 
stripped before adding to the database.

Original comment by funnyfin...@hotmail.com on 22 Jun 2009 at 4:52

GoogleCodeExporter commented 9 years ago
Yes, when inserting strings into MySQL obblm uses php escaping functions, have 
a look
for yourself..

Original comment by Nimda...@gmail.com on 22 Jun 2009 at 7:57

GoogleCodeExporter commented 9 years ago
Dont fix something that's not broken...Please show an SQLinjection example for 
OBBLM
where you can get data you're not supposed to see from DB. Like 
usernames/passwords.
(Or overwriting data that you shouldn't have write access to.)

Naming a team as per your example doesn't destroy anything, right? (I think the 
guys
that made mysql_escape_*() know what they do.)

Original comment by blodae@gmail.com on 23 Jun 2009 at 9:56

GoogleCodeExporter commented 9 years ago
I agree in all ways.

Original comment by Nimda...@gmail.com on 23 Jun 2009 at 10:19

GoogleCodeExporter commented 9 years ago
I am doing this for my input.  I've already secured all my integers with the 
XSD 
(haven't uploaded the XSD validation yet)  I am fine with the strings and I 
wrap 
them with the mysql escape in my latest (no uploaded as well).

The way to break it isn't easy.  It would probably involve creating a form 
submittal 
and using the integer values to inject the code as the strings may be safe with 
the 
mysql escape.  I am no sql guru by any stretch so I probably don't know how to 
break 
it, but I know how to restrict what is uploaded via leegmgr.

Original comment by funnyfin...@hotmail.com on 23 Jun 2009 at 10:52

GoogleCodeExporter commented 9 years ago
I uploaded my changes for the leegmgr upload.  I can go ahead an close this as 
fixed 
for leegmgr.  Just let me know.  Thanks.

Original comment by funnyfin...@hotmail.com on 23 Jun 2009 at 11:07

GoogleCodeExporter commented 9 years ago
I'll go ahead and close this as I added the validation for the XML submitted to 
leegmgr.

Original comment by funnyfin...@hotmail.com on 25 Jun 2009 at 6:18