Does this work to stop sql injections

Question

I have been using the block of code below to supposedly stop sql injections. It is something someone showed me when I first started php(which was not that long ago)

I place it in every page just as shown on the open. I am wondering if it is effective? I do not know how to test for sql injections

<?php

//Start the session

session_start();


//=======================open connection

include ('lib/dbconfig.php');

//===============This stops SQL Injection in POST vars

  foreach ($_POST as $key => $value) {
    $_POST[$key] = mysql_real_escape_string($value);
  }

  foreach ($_GET as $key => $value) {
    $_GET[$key] = mysql_real_escape_string($value);
  }

My typical insert and update queries look like this

$insert = ("'$email','$pw','$company', '$co_description', '$categroy', '$url', '$street', '$suite', '$city', '$state', '$zip', '$phone', '$date', '$actkey'");

mysql_query("INSERT INTO provider (email, pw, company, co_description, category, url, street, suite, city, state, zip, phone, regdate, actkey) VALUES ($insert)") or die ('error ' . mysql_error());

mysql_query("UPDATE coupon SET head='$_POST[head]', fineprint='$_POST[fineprint]', exdate='$exdate', creationdate=NOW() WHERE id='$cid'") or die ('error ' . mysql_error());

Answer 1

That's somewhat effective, but it's suboptimal -- not all of the data you receive in _GET and _POST will go into the database. Sometimes you might want to display it on the page instead, in which case mysql_real_escape_string can only hurt (instead, you'd want htmlentities).

My rule of thumb is to only escape something immediately before putting it into the context in which it needs to be escaped.

In this context, you'd be better of just using parameterized queries -- then escaping is done for you automatically.

Answer 2

This is not enough. 1. You're missing cookies, $_COOKIE variable. 2. If you use $_REQUEST you're in trouble. 3. You didn't show your queries, you must enquote each variable with single quotes '' when you put it into query (especiall when the data is supposted to be an integer and you might think that quote is not necessary in that case, but that would be a big mistake). 4. Data used in your query could come from other source.

The best way is to use data binding and have the data escaped automatically by the driver, this is available in PDO extension.

Example code:

$PDO = new PDO('mysql:dbname=testdb;host=127.0.0.1' $user, $password);
$stmt = $PDO->prepare("SELECT * FROM test WHERE id=? AND cat=?");
$stmt->execute(array($_GET["id"], $_GET["cat"]));
$rows = $stmt->fetchAll(PDO::FETCH_ASSOC);

You can also bind data using string keys:

$stmt = $PDO->prepare("SELECT * FROM test WHERE id = :id AND cat = :cat");
$stmt->execute(array(":id" => $_GET["id"], ":cat" => $_GET["cat"]));

If you want to learn PDO, you might find useful these helper functions I use:

http://www.gosu.pl/var/PDO.txt

PDO_Connect(dsn, user, passwd) - connects and sets error handling.
PDO_Execute(query [, params]) - only execute query, do not fetch any data.
PDO_InsertId() - last insert id.

PDO_FetchOne(query [, params]) - fetch 1 value, $count = PDO_FetchOne("SELECT COUNT(*) ..");
PDO_FetchRow(query [, params]) - fetch 1 row.
PDO_FetchAll(query [, params]) - fetch all rows.
PDO_FetchAssoc(query [, params]) - returns an associative array, when you need 1 or 2 cols

1) $names = PDO_FetchAssoc("SELECT name FROM table");
the returned array is: array(name, name, ...)

2) $assoc = PDO_FetchAssoc("SELECT id, name FROM table")
the returned array is: array(id=> name, id=>name, ...)

3) $assoc = PDO_FetchAssoc("SELECT id, name, other FROM table");
the returned array is: array(id=> array(id=>'',name=>'',other=>''), id=>array(..), ..)

Each of functions that fetch data accept as 2nd argument parameters array (which is optional), used for automatic data binding against sql injections. Use of it has been presented earlier in this post.

Answer 3

Kind of.

The mysql_real_escape_string function takes the given variable and escapes it for SQL queries. So you can safely append the string into a query like

$safe =  mysql_real_escape_string($unsafe_string);
$query = 'SELECT * FROM MyTable WHERE Name LIKE "' . $safe . '" LIMIT 1';

It does NOT protect you against someone putting malicious code into that query to be displayed later (i.e. XSS or similar attack). So if someone sets a variable to be

// $unsafe_string = '<script src="http://dangerous.org/script.js"></script>'
$safe =  mysql_real_escape_string($unsafe_string);
$query = 'UPDATE MyTable SET Name = "' . $safe . '"';

That query will execute as you expect, but now on any page where you print this guy's name, his script will execute.

Answer 4

This is completely WRONG approach.

In fact, you are mimicking infamous magic quotes, which is acknowledged as a bad practice. With all it's faults and dangers.

• To help you understand why your initial way was wrong Magic quotes in PHP
• To help you understand why escaping has nothing to do with "data safety" yet not sufficient to protect your query: Replacing mysql_* functions with PDO and prepared statements
• To help you understand when prepared statements not sufficient either and what to do in these cases: In PHP when submitting strings to the database should I take care of illegal characters using htmlspecialchars() or use a regular expression?

Answer 5

this is not to prevent SQL Injection the real escape method only add \ to the dangerous

characters like " or ' so a string with "hi"do'like" will become "hi\"do\'like\" so it is

less dangerous

this method is not always usefull ; in case you want to display the content of tha escaped

variable in a page it will only destroy it and make it less readable