0

I am quite new to PHP and have been told my MySQL statements are insecure from injections.

There are my old queries:

$addbook = "INSERT INTO bookings (bookID, startDate, startTime, endDate, endTime) ";
$addtempres .= "VALUES ('".$bookid."', '".$startdate."', '".$starttime."', '".$enddate."', '".$endtime."')";
$insertBook = mysql_query($addbook);

$getblogposts = mysql_query("SELECT * FROM blogposts WHERE deleted = 'no' ORDER BY postID DESC LIMIT 4");

After reading about it I understand they are insecure, I also understand that mysql_query is old, and being depreciated.

I have however written a lot of these queries and the realization that they are all old and unsafe is daunting, so I started trying to secure them.

So I did this:

$escapedbookid = mysql_real_escape_string($bookid) ;
$escapedstartdate = mysql_real_escape_string($sqlcoldate);
$escapedstarttime = mysql_real_escape_string($forstarttime);
$escapedenddate = mysql_real_escape_string($sqlretdate);
$escapedendtime = mysql_real_escape_string($forendtime);
$escapedactive = mysql_real_escape_string('false');

$addtembook = "INSERT INTO bookings (bookID, startDate, startTime, endDate, endTime) ";
$addtempres .= "VALUES ('".$escapedbookid."', '".$escapedstartdate."', '".$escapedstarttime."', '".$escapedenddate."', '".$escapedendtime."')";
$insertRes = mysql_query($addtempbook);

Is this more secure? I appreciated PDO prepared statements are easier and more secure when I learn to translate my current queries into them, but I was just wondering if what I am doing is making things more secure or not?

insertusernamehere
  • 23,204
  • 9
  • 87
  • 126
AltTab
  • 65
  • 2
  • 7
  • 2
    `mysql_` code will soon be deprecated for many reasons, security being one of them. I highly recommend you look into mysqli: http://us2.php.net/manual/en/mysqli.overview.php – Andy Mar 14 '13 at 11:50
  • 1
    [**Please, don't use `mysql_*` functions in new code**](http://bit.ly/phpmsql). They are no longer maintained [and are officially deprecated](https://wiki.php.net/rfc/mysql_deprecation). See the [**red box**](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. If you choose PDO, [here is a good tutorial](http://www.brightmeup.info/article.php?a_id=2). – insertusernamehere Mar 14 '13 at 11:52
  • 1
    @Andy does mysqli make things more secure as well, or is it just about keeping up with best practice? – AltTab Mar 14 '13 at 11:53
  • 1
    @TryingToBeZen Yes, mysqli and PDO are much more secure than mysql_ – Andy Mar 14 '13 at 11:54
  • @insertusernamehere Thank you, I will review all that, but until I familiarize myself with PDO properly, is the way i'm using mysql_real_escape_string making things a little bit more secure? JUST until I have the time to rewrite all my queries? – AltTab Mar 14 '13 at 11:58
  • @TryingToBeZen you'd be locking the door instead of leaving it wide open, but someone could still get in if they really wanted to. – bcmcfc Mar 14 '13 at 12:00
  • 1
    Here's a post that might help: [How to prevent SQL injection in PHP?](http://stackoverflow.com/q/60174/1456376). The second answer and the comments cover the `mysql_real_escape_string` topic too. – insertusernamehere Mar 14 '13 at 12:05

1 Answers1

1
  1. Stop using the mysql_ library - it is deprecated - use either mysqli_ library instead (or PDO).
  2. Look at the web page for prepared statements - http://www.php.net/manual/en/mysqli.prepare.php - The example will give you a template on how to write the code safe from SQL injections.
insertusernamehere
  • 23,204
  • 9
  • 87
  • 126
Ed Heal
  • 59,252
  • 17
  • 87
  • 127