SQL Injection protection

malfist

I have a website that stores some limited data on users (login name, password (sha1 encrytion), email, etc) and I have a sign on. To check the password and username it sends a SELECT query (where username= input) to the database. The only thing I do to the input is trim it and remove html chars (replace them with the unicode (htmlspecialchars()). Is it vurnrable to SQL injections? If so how would I prevent them? Search the string for simicolin's and remove them or do I need to do more?

Malfist

PS Great site, I just joined.

Sunday_Ironfoot

What server side technology are you using (PHP, ASP.NET etc)? Also what database? Basically you'll need to deal with single quote characters if it's a string, and if it's a numeric value check that the actual value is numeric and that someone hasn't tried to enter '-- DROP DATABASE; -- or something like that.

BTW, you ought the escape HTML characters on the way out of the database before before display on screen, rather than as they go in. Unless you want to disallow HTML chars for passwords and usernames, but even then you'll still need to check them on the way out.

malfist

I'm useing a LAMP (Linux, Apache, MySQL, PHP), the codeing is done in CSS, XHTML, and PHP for the site with the MySQL database. It really shouldn't matter with the password what I change the html characters to as long as it's consistent. So, just check for quote characters and simicolons?

Whiskey_Tango_Foxtro

[quote user="malfist"]I'm useing a LAMP (Linux, Apache, MySQL, PHP), the codeing is done in CSS, XHTML, and PHP for the site with the MySQL database. It really shouldn't matter with the password what I change the html characters to as long as it's consistent. So, just check for quote characters and simicolons?[/quote]

I *highly* suggest you read the PHP manual, or some other form of comprehensive LAMP tutorial/book. There already exists a function for you that does exactly what you want:

PHP: mysql_escape_string - Manual

Edit:

Whoops, that's depricated. Can you tell it's been a few years since I did LAMP stuff? Before it was called LAMP, that is. Try using this, instead:

PHP: mysql_real_escape_string - Manual

malfist

Thank you, thank you very much. I'm just wondering why my two PHP books didn't mention it? One was even called PHP and MySQL! Lovely, it went on and on about 'magic quotes' I wouldn't recomend it.

Malfist

P.S. PHP for Dummies didn't say anything about it either, not even in it's "database" chapter. Lovely.

malfist

I went to it (my website) before I updated it and set the password box to ' OR ''=' and the username to admin, but it told me that it was an incorrect password. Shouldn't it allow me to log in with that?

EDIT: nvm, I only send the username in the select query to the database. Oops

whalemangler

// this works in .net/sql server land, never used a LAMP :)

invulnerableString = vulnerableString.Replace("'", "''"); // replace single quotes with 2 single quotes (escaping them)

the correct way isn't adding slashes yourself or adding quotes yourself.

function quote_smart($string){

if(get_magic_quotes_gpc())

$string = stripslashes($string);

if(!is_numeric($string))

$string = "'" . mysql_real_escape_string($string) . "'";

return $string;

}

note that this only works with an open connection, as mysql_real_escape_string checks the version of the server you're connected to to do proper escaping.

Noisy_Crow

If you can't used parametized stored procedures, they your best bet is to use a white-list of acceptable characters. Strip out anything that isn't in the white list before accepting any user-provided data. For any normal purpose, A-Z, a-z, 0-9, plus selected punctuation (shifted number keys plus period, comma, and question mark) should suffice.

kasm

If you want to be pretty much completely immune to SQL injection, you can use prepared statements (parameter queries). I would read the PHP manual for mysqli, or switch to using PDO

mysqli:

http://usphp.com/manual/en/ref.mysqli.php

PDO:

http://usphp.com/manual/en/ref.pdo.php

Edit: Forum software makes me cry

whalemangler

Stripping data isn't a good idea in many cases.

What if I want to search for Dave's Burger Joint?

writejustify

[quote user="whalemangler"]

Stripping data isn't a good idea in many cases.

What if I want to search for Dave's Burger Joint?

[/quote] Chances are that a login and password don't need apostrophes. ;) Those are the best places for sql injection as people with valid login ids are less likely to be trying to sql inject your site, at least while logged in.

But you make a good point. What I've usually done is limit permissable characters for most fields and have extra coding for search fields and company names.

rdrunner

[quote user="whalemangler"]

Stripping data isn't a good idea in many cases.

What if I want to search for Dave's Burger Joint?

[/quote]

It will work.. Since "Daves Burger joint" is also stored stripped in the DB :D

Anyway...

You should not strip data. Especially in a password field where the user usually does not see what he just typed. There are ways to escape those strings in a propper manner...

Monkeyget

[quote user="writejustify"]Chances are that a login and password don't need apostrophes. ;) Those are the best places for sql injection as people with valid login ids are less likely to be trying to sql inject your site, at least while logged in.

But you make a good point. What I've usually done is limit permissable characters for most fields and have extra coding for search fields and company names.

[/quote]

With that kind of reasoning you end up with crap like Poor Mr. O'Hare :

http://thedailywtf.com/forums/thread/87226.aspx

Irrelevant

You want MySQL Improved's parametrised queries.

(This from a perl coder who just discovered to his horror that the PHP standard library doesn't seem to do parametrised queries... shudder)

Tweenk

PHP standard library doesn't do anything DB-related. In fact, in PHP there is no concept of a standard library, only of "core functions" - i.e. tokenizer functions, str*, shell_exec, array functions etc. It's the php_mysql extension that doesn't handle parametrized queries, because its API was designed for antique versions of MySQL (it's one of the oldest extensions) that didn't have this facility. Some people think of mysql_* and some other functions as belonging to "standard library", because it used to be bundled with all distributions of PHP and enabled by default. As of PHP 5 it's no longer true, since all extensions were moved to PECL.

As for the SQL injection problem, it's best to read this exact topic in the PHP manual. The essential code snippet has already been posted in this thread.