Page 1 of 3

[56K WARN (page 2)] Newbie needs help on SQL injection

Posted: Fri Aug 19, 2005 4:39 am
by sleazyfrank
Hi there - I'm in my first week of coding in php and it's going great so far. I'm developing a booking system for my company and there are two pages; one displays the courses and dates they are available in the year and the second is a generic booking page that, when you click the Book this course link on the first page which contains the name of the table, which half of the year the user is currently looking at and the record id, takes the querystring and uses it to construct a sql query to pull out the right data from the mysql db. Now it's working fine except a fellow programmer took a look at the booking page and said it was at risk from sql injection attacks.

I've had a look around and found some info on this and that you have to escape the characters so how but after that I am pretty lost.

This is the code:

Code: Select all

//get courseID
					$courseID = $_GET['courseID'];
					//get table for query
					$tableName = $_GET['table'];
					//get which half of the year
					$whichHalf = $_GET['whichHalf'];
					//set up query
					$query = "SELECT * FROM ". $tableName . " WHERE RecordID = " . $courseID;
					//echo $query . "<br>";
					//connect to database and retrieve record
					
					//connect to db
					mysql_connect($host,$username,$password);
					@mysql_select_db($database) or die("Unable to select database");
Obviously I've declared my server details but haven't posted them here. As you can see I GET three variables, the courseID, the tableName and whichHalf which is used to determine which half yearly set of months to get from the db. But as you can see my query is exposed. But all the tutorials and help I have seen on line about solving sql injections talk about queries that go SELECT * FROM sometablename WHERE RecordID = " $blahblah; - so does anyone have any ideas of how to make my page more secure and how to sort my query out so it's safe?

thanks for any comments or help

frank

Posted: Fri Aug 19, 2005 5:22 am
by shiznatix
its probebly a really bad idea to use get variables in a query becuase those can be realllly easily tampered with. using post would be best and then use mysql_real_escape_string() around the variables and you should be golden

Posted: Fri Aug 19, 2005 5:39 am
by sleazyfrank
Hi thanks for the reply - I am only in my 1st week of programming in PHP so a lot of what you said didn't mean much. Sorry! :) Could you possibly reexplain for my inexperienced brain or provide a code example?

Am I correct in doing my link as <a href="bookcourse.php?table=coursesUnixLinux&courseID=<? echo $link ?>&whichHalf=<? echo $whichHalf ?>Book course</a> ?

thanks!

frank

Posted: Fri Aug 19, 2005 6:10 am
by dbevfat

Posted: Fri Aug 19, 2005 6:43 am
by raghavan20
your syntax looks fine except some missing semicolons.
a suggestion, try using urlencode on url variables.
ex:

Code: Select all

<a href="bookcourse.php?table=<?php echo urlencode("coursesUnixLinux"); ?>&courseID=<?php echo urlencode($link );>&whichHalf=<?php echo urlencode($whichHalf ); ?>">Book course</a> ? "
while using GET or POST, use urldecode($_GET["$var"])

Posted: Fri Aug 19, 2005 6:52 am
by patrikG
Neither urldecode() nor mysql_real_escape_string() will remove the threat of SQL-injection. You want to allow only valid characters.

If you only want to allow alphanumeric characters use something like

Code: Select all

function filterAlphanumeric($string){
      return preg_replace("/[^a-zA-Z0-9]/", "", $string);
}
It strips any character that's not alphabetical or numerical from the string and returns it.

Using that function your code could look like

Code: Select all

//get courseID
               $courseID = filterAlphanumeric($_GET['courseID']);
               //get table for query
               $tableName = filterAlphanumeric($_GET['table']);
               //get which half of the year
               $whichHalf = filterAlphanumeric($_GET['whichHalf']);
               //set up query
               $query = "SELECT * FROM ". $tableName . " WHERE RecordID = " . $courseID;
               //echo $query . "<br>";
               //connect to database and retrieve record
               
               //connect to db
               mysql_connect($host,$username,$password);
               @mysql_select_db($database) or die("Unable to select database");

Posted: Fri Aug 19, 2005 7:25 am
by raghavan20
that preg_replace by patrick would help counter sq injection.
I was commenting only about the validity of his url syntax.
patrick, i hope you are replacing all chars other than the ranges, [a-z], [A-Z], 0-9. do all preg_replace do a global search or act on multiple occurences cos in javascript we use this /g modifier. :roll:

Posted: Fri Aug 19, 2005 7:27 am
by patrikG
Using Javascript to prevent SQL injection is pointless, you can easily disable it. You need a serverside solution.

Posted: Fri Aug 19, 2005 7:33 am
by neophyte
If you are expecting an integer in your query string, I like to use is_numeric() on it.

Posted: Fri Aug 19, 2005 7:35 am
by shiznatix
is_numeric() does not always give correct results becuase if there is any number anywhere in the string it returns true which makes no sence

Posted: Fri Aug 19, 2005 7:40 am
by sleazyfrank
LOL - hey people remember I am a PHP virgin here! Less than 7 days experience!! I've taken patrikG's code and I'm trying it out now to see what happens.

thanks!

Frank

Posted: Fri Aug 19, 2005 7:54 am
by patrikG
sleazyfrank wrote:LOL - hey people remember I am a PHP virgin here! Less than 7 days experience!!
This is a discussion forum - things are being discussed here ;)

Posted: Fri Aug 19, 2005 8:15 am
by sleazyfrank
Oki doki - I'll leave you php experts to fight it out, but your code seems to be working fine, well it hasn't interefered with the form. I'll have to get it tested for sql injection. Cheers for help.

BTW I was looking for a good manual to get into php and mysql - I was looking at the visual quickpro 'PHP and MySQL for dynamic websites' by Larry Ullman. Thoughts?

ta

Frank

ps patrikG you're in sussex too??

Posted: Fri Aug 19, 2005 8:20 am
by patrikG
sleazyfrank wrote:Cheers for help.
No worries. ;)
sleazyfrank wrote: BTW I was looking for a good manual to get into php and mysql - I was looking at the visual quickpro 'PHP and MySQL for dynamic websites' by Larry Ullman. Thoughts?
Unless it covers object oriented programming in good detail, I don't think any beginner's PHP book is worth looking at. I usually recommend Harry Fuecks "PHP Anthology".
sleazyfrank wrote:ps patrikG you're in sussex too??
Yup. And I used to work in Horsham for a couple of months some time ago... We always went to the Malt & Shovel :)

Posted: Fri Aug 19, 2005 8:27 am
by feyd
shiznatix wrote:is_numeric() does not always give correct results becuase if there is any number anywhere in the string it returns true which makes no sence
a-hem:

Code: Select all

[feyd@home]>php -r "var_export(is_numeric('asdf1234'));"
false
[feyd@home]> _