The Daily WTF: Curious Perversions in Information Technology

2013-06-14 Reply Admin

luptatum:
Apostle:
TRWTF is that Jon didn't explain how to fix the problem, he just explained what the problem was.
I'm actually surprised he and you are that forgiving. If I was Jon and if I saw this, they'd be some heads flying out of the building. I'm not gonna waste my time with idiots who don't understand basic stuff.

Actually, I wouldn't have shown him the "proof-of-concept". I would have simply told him that it's vulnerable to an SQL injection attack, and noted his response. If it's along the lines of "what's that?", then he's out the door. If it's along the lines of "I know, but..." then it depends on the "but". (Though I'm not sure there are any acceptable "but"s.) If he argues "no, it's not", then show him the proof-of-concept.

2013-06-14 Reply Admin

Jayne:
Dear Diary,
Today I learned that the purpose of TDWTF is to afford developers the opportunity to bitch about everything about the stories.

Simon.

That's a pretty stupid comment. What kind of moron says things like that? You are TRWTF.

:-)

2013-06-14 Reply Admin

SeanC:
I can't log in...
My password is Hello@Wor;d_

Fortunately, the passwords are all stored plain-text, so a simple pass through the table, removing the "illegal" characters from the passwords would suffice. (And the users would never know, since they can still use the "illegal" password and log in.)

2013-06-14 Reply Admin

Ken B:
SeanC:
I can't log in...
My password is Hello@Wor;d_

Fortunately, the passwords are all stored plain-text, so a simple pass through the table, removing the "illegal" characters from the passwords would suffice. (And the users would never know, since they can still use the "illegal" password and log in.)

Isn't it illegal to store passwords in plain text?

Monomelodies · 2013-06-14 Reply Admin

pscs:
2) if you are using it where the data is a number, then it won't help - only use it for escaping strings. Force number inputs to be numeric instead.

I'd argue that's more a problem in the way the SQL is written - AFAIK every major database supports quoted numerical values, thus solving the problem (IF you write "proper" SQL). Prepared statements in at least PDO (can't comment on other techniques in other languages, but I'm sure they're similar) quote numerical values automatically too.

I recall there's a few exceptions like in doing column = column & ~1 to flip a bit, at least in PostgreSQL. Though it's been a while since I had to work around those, might be fixed by now. But in any case these are relatively uncommon operations and should be handled by something framework-ish anyway IMHO.

2013-06-14 Reply Admin

pscs:
50% Opacity:
Valczir:
For that matter, all you php devs, mysql_real_escape_string doesn't solve anything.

I would really like to hear a clarification to this statement. Why does it not solve anything? Can you demonstrate a problem with code using mysql_real_escape_string?

There seems to be a lot of FUD here from 'prepared statement fanatics'.

Yes, prepared statements are the 'way to go', but if you have legacy code, you can stop SQL injection attacks without totally rewriting your code.

You can stop injection attacks if you are careful using mysql_real_escape_string and sprintf (or similar)

I've spent a while Googling this (since people were saying 'Google it, and you'll find plenty of examples of people telling you to use prepared statements' - well, that's true, but no one has said that mysql_real_escape_string won't work, just that it's not the best solution.

The reasons NOT to use mysql_real_escape_string AFAICS are:

it relies on you catching all places where you need to use it

if you are using it where the data is a number, then it won't help - only use it for escaping strings. Force number inputs to be numeric instead.

eg for (2), if you have

'SELECT * FROM users WHERE usernumber=' . mysql_real_escape_string($_REQUEST['usernumber']) . ';';

then it won't help at all - because you are expecting $_REQUEST['usernumber'] to be a number, but it may be a string, so if it's: "3 OR 1=1", then the mysql_real_escape_string won't do a thing, but you've still got an injection attack.

To get around this, you could use sprintf (with '%d') or some other way of coercing the parameter to a number before adding it into your query string.

So, mysql_real_escape_string will work fine, as long as it is used everwhere, and properly, by someone who understands what they're doing. If you are writing new software, using prepared statements is probably the way to go (unless your web host's PHP doesn't support PDO), because you're less likely to make mistakes.

I haven't seen anyone (except here) say that mysql_real_escape_string won't work IF USED PROPERLY.

I've read about vulnerabilities with mysql_real_escape_string being bypassed by non-utf8 characters. This may have been addressed in updates to php, but mysql_ functions have been deprecated as well. http://stackoverflow.com/questions/5139127/php-sql-injection-utf8-poc

The current most accepted answer is use PDO with parameterized queries. I have yet to hear of a proof of concept of SQL injection bypassing properly parameterized PDO queries. If anyone know of such a thing, please post a link!

joelkatz · 2013-06-14 Reply Admin

FragFrog:
Technically, it is possible the password was hashed client-side before posting it.

No, that's not possible. The "password" is whatever the client needs to send to the server to get access. Any hashing done on the client side would, by definition, be hashing of something other than the password. The result of the hash would be the password because that's what the client needs to gain access to the server.

2013-06-14 Reply Admin

Do monkeys eat peanuts, now? I'd figure if you pay peanuts, you get elephants, or perhaps squirrels.

2013-06-14 Reply Admin

Cool, i didn't see that coming!! :-)

2013-06-14 Reply Admin

For God's sake man, sanitize your inputs... Or alternatively, use parameterized inputs. Always.

2013-06-14 Reply Admin

lolatu:
I've read about vulnerabilities with mysql_real_escape_string being bypassed by non-utf8 characters. This may have been addressed in updates to php, but mysql_ functions have been deprecated as well. http://stackoverflow.com/questions/5139127/php-sql-injection-utf8-poc
The current most accepted answer is use PDO with parameterized queries. I have yet to hear of a proof of concept of SQL injection bypassing properly parameterized PDO queries. If anyone know of such a thing, please post a link!

See here: http://stackoverflow.com/questions/12703420/shortcomings-of-mysql-real-escape-string

And no, Akismet, this is a perfectly helpful comment. I mean: FRIST!

2013-06-14 Reply Admin

pscs:
There seems to be a lot of FUD here from 'prepared statement fanatics'.
Yes, prepared statements are the 'way to go', but if you have legacy code, you can stop SQL injection attacks without totally rewriting your code.

You can stop injection attacks if you are careful using mysql_real_escape_string and sprintf (or similar)

I've spent a while Googling this (since people were saying 'Google it, and you'll find plenty of examples of people telling you to use prepared statements' - well, that's true, but no one has said that mysql_real_escape_string won't work, just that it's not the best solution.

The reasons NOT to use mysql_real_escape_string AFAICS are:

it relies on you catching all places where you need to use it

if you are using it where the data is a number, then it won't help - only use it for escaping strings. Force number inputs to be numeric instead.

eg for (2), if you have

'SELECT * FROM users WHERE usernumber=' . mysql_real_escape_string($_REQUEST['usernumber']) . ';';

then it won't help at all - because you are expecting $_REQUEST['usernumber'] to be a number, but it may be a string, so if it's: "3 OR 1=1", then the mysql_real_escape_string won't do a thing, but you've still got an injection attack.

To get around this, you could use sprintf (with '%d') or some other way of coercing the parameter to a number before adding it into your query string.

So, mysql_real_escape_string will work fine, as long as it is used everwhere, and properly, by someone who understands what they're doing. If you are writing new software, using prepared statements is probably the way to go (unless your web host's PHP doesn't support PDO), because you're less likely to make mistakes.

I haven't seen anyone (except here) say that mysql_real_escape_string won't work IF USED PROPERLY.

Thank you, couldn't have said it better myself. The FUD's killing me sometimes.

Coyne · 2013-06-14 Reply Admin

C-Derb:
Coyne:
x:
tempUserName= Frist" or "1==1 tempPassword= Frist" or "1==1
Passes the filter nicely

So what?
tempUserName = tempUserName.Replace("&", "")
tempUserName = tempUserName.Replace("#", "")
tempPassword = tempPassword.Replace("&", "")
tempPassword = tempPassword.Replace("#", "")
...and another contracting fee. No problem.
Fail.

You are continuing with the wrong solution. If you want to disallow special characters in the username an password, you shouldn't filter them out by replacing them with spaces. You should just reject them.

Put it this way. Say you implement your suggested code. Now you are going to invoke a database call to see if a record exists that you already know doesn't exist (assuming you've implemented similar logic when storing the username/password). So someone who tries to actually submit the password above is going to have your code check for
701141051151163432111114323449616149
which won't match any record, unless that really is the person's password, in which case they won't be typing the &'s and #'s in the first place!!!

I know: I was doing a parody of the original developer. Who is too lame to be in this business.

2013-06-15 Reply Admin

C-Derb:
iWantToKeepAnon:
3) His solution isn't that bad and probably secures against most robot/crawler attacks. Looking at the code "upgrade" doesn't make it obvious how to inject into it now.
At the risk of feeding a troll....
Just. Stop. His solution certainly is "that bad".

Look, I can forgive the initial code that contains a clear SQL Injection vulnerability, assuming lack of experience. But what is unforgivable is then going about solving the problem without learning more about it. Even if he still didn't know anything about the term "SQL Injection", an hour on Google surely would have led to copious examples of similarly flawed code with plenty of recommendations to implement parameterized queries, prepared statements, etc.

The problem is that for people who suck at coding, string manipulation is easy, safe and comfortable. That's why they will actually prefer it to simpler solutions like parameter substitution.

In fact, I'd go so far as to say if I've got to interview a candidate, I'm going to ask a number of programming problems and see if they go for string manipulation. People who reach for that tool first don't get objects, don't get abstractions, and are going to fucking suck and I will want to punch them in the throat.

2013-06-15 Reply Admin

QA:
Geoff:
Apostle:
TRWTF is that Jon didn't explain how to fix the problem, he just explained what the problem was.

So Jon should pay good money to have work done for him; and then still do it himself? Sorry no; Jon was provided with a defective product and the provider should make it right.

If you bought a new car and discovered it only starts 7 in 10 times you turn the key; you would not do a complete tear down diagnose the problem and then take it back to the dealer with a proposed repair in mind. No you'd just take it back to the dealer show them problem and say "fix it".

Turn key, car started. Unable to reproduce. Resolution: Fixed

Not quite:

Started car 67 times.

On attempts 58 through 67, car started correctly.

Car now starts 10 out of 10 times.

On attempt 67, starter was sluggish.

Checked battery levels, found battery voltage low.

Replaced battery. Key bent. Replacement key ordered.

Resolution: fixed.

2013-06-16 Reply Admin

This code is .NET, .NET does not have a function "addslashes" in both examples (before and after the "fix") the text is being read from the text property of the textbox control on the page. He should be using a stored procedure with SQL parameters.

xaade · 2013-06-19 Reply Admin

Apostle:
TRWTF is that Jon didn't explain how to fix the problem, he just explained what the problem was.

THANK YOU!!!!!!

I hear so much about SQL injection and how stupid people are for not anticipating it, that I, with very little web development experience, am paranoid at the concept of attempting to build a site.

Sure I could do a Google search for how to prevent SQL injection, but how should I trust them? I mean, I've seen people attempt to post answers and then get ridiculed for not thinking of possibility #445.

Without doing any research, my immediate thought is that there is no correct way and everyone's a critic.

chubertdev · 2013-06-19 Reply Admin

xaade:
Apostle:
TRWTF is that Jon didn't explain how to fix the problem, he just explained what the problem was.

THANK YOU!!!!!!

I hear so much about SQL injection and how stupid people are for not anticipating it, that I, with very little web development experience, am paranoid at the concept of attempting to build a site.

Sure I could do a Google search for how to prevent SQL injection, but how should I trust them? I mean, I've seen people attempt to post answers and then get ridiculed for not thinking of possibility #445.

Without doing any research, my immediate thought is that there is no correct way and everyone's a critic.

I think that would be the case if it were a co-worker, not a contractor.

2013-06-21 Reply Admin

x:
tempUserName= Frist" or "1==1 tempPassword= Frist" or "1==1
Passes the filter nicely

You do realize that you're trying to break into a SQL engine, not a web browser, right?

2013-06-25 Reply Admin

Why? Client side hashing along with a session bound random salt provided by the server is an excellent way to ensure security on an untrusted network where sniffing can occur. Certainly you don't want to transmit clear text passwords over the internet. Now of course you could say, "why not just use SSL?" And of course you should, but there is nothing wrong with an additional layer of security.

2013-06-25 Reply Admin

For the kind of stuff that I do, I use Sublime Text 2. I think it's better, you might not. It's all very subjective anyways. I for example am not a big fan of PHP, I prefer javascript (node.js) and python, but I wouldn't touch anything Microsoft with a 10 foot pole.

SQL Injection: What's That?

Leave a comment on “SQL Injection: What's That?”