-2

I know most of the answers will say use PDO/Mysqli but I'm trying to see if I can do it this way then move on to PDO/Mysqli still learning:

Will this function be enough to prevent mysql injection?

function anti_inject($sql)
{

    $sql = preg_replace(sql_regcase("/(from|select|insert|delete|where|drop table|show tables|#|\*|--|\\\\)/"), "", $sql);
    $sql = preg_replace("/[^a-zA-Z0-9]+/", " ", $sql);
    $sql = mysql_real_escape_string($sql);
    $sql = trim($sql);
    $sql = strip_tags($sql);
    $sql = addslashes($sql);
    $sql = strtolower($sql);
    return $sql;
}

Looking for a better replacement for this line $sql = preg_replace(sql_regcase("/(from|select|insert|delete|where|drop table|show tables|#|*|--|\\)/"), "", $sql);

As I do want to check for names that have "from" "select" "insert" gaming tags etc

I've disabled drop table from the mysql user

Swaly
  • 89
  • 8
  • 1
    Why are you learning to use the deprecated, obsolete and generally bad library before learning to use a modern one? – Quentin Aug 18 '13 at 11:39
  • 4
    This looks like hitting every button on the TV remote just to change the channel. You may have changed the channel but you may have also started the DVD player and turned the image from color to black and white. Better to check the remote for the correct button first. (I.e. read the documentation about [SQL injection](https://en.wikipedia.org/wiki/SQL_injection)!) – Tomas Creemers Aug 18 '13 at 11:43

2 Answers2

1

No. That's hopeless.

$sql = preg_replace(sql_regcase("/(from|select|insert|delete|where|drop table|show tables|#|\*|--|\\\\)/"), "", $sql);

This throws away data for no apparent reason

$sql = preg_replace("/[^a-zA-Z0-9]+/", " ", $sql);

This throws away data for no apparent reason

$sql = mysql_real_escape_string($sql;

If you hadn't lost the ), then this is the correct way to convert a piece of data for inserting into an SQL query. 

$sql = trim($sql);

This throws away data for no apparent reason

$sql = strip_tags($sql);

This throws away data for no apparent reason

$sql = addslashes($sql);

This escapes data in a way inappropriate for an SQL query. Some data will be double escaped (and therefore broken) because you have already used mysql_real_escape_string.

$sql = strtolower($sql);

This mangles data for no apparent reason.

When using the obsolete, deprecated mysql_ library, mysql_real_escape_string is the only escaping function you should use. You then need to take the results and use them appropriately when bashing your string of SQL together.

Don't use mysql_ though. Use PDO and parameterised queries.

Community
  • 1
  • 1
Quentin
  • 914,110
  • 126
  • 1,211
  • 1,335
  • I needed strtolower, I use this to check for names on a java-based game, I only allow numbers & letters, and I find it easier to make all chars lower incase a user checks the name "isSoFly"for example; so 2 of the functions have a reason the rest is what I've been reading. – Swaly Aug 18 '13 at 11:43
  • @Swaly beside reading one is supposed to *understand* what they read, mind you. – Your Common Sense Aug 18 '13 at 11:46
  • @Swaly speaking of lowercase (which has nothing to do with whatever safety and thus offopic), mysql already has a case-independent comparison. – Your Common Sense Aug 18 '13 at 11:47
  • Looking at the way you are using mysql, not "mysqli_ or PDO" but **PDO prepared statements** only – Your Common Sense Aug 18 '13 at 11:48
1

Strictly speaking, in terms of SQL injection protection this function is awfully useless and unusable at the same time. While it may serve you in some particular case of yours, it cannot be used as a general purpose solution. Yet even for such a case it is awfully redundant.

So, why not to use a solution already proved to be error-proof for all the (covered) cases - PDO prepared statements?

Community
  • 1
  • 1
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • Say I use PDO, does the actual query change or only the code? like this for example; Will this [mysql_query("UPDATE items SET `name` = '$newname' WHERE `index` = $index LIMIT 1");] change to [$pdo->mysql_query("UPDATE items SET `name` = '$newname' WHERE `index` = $index LIMIT 1");] or is it different then that? – Swaly Aug 18 '13 at 12:01
  • 1
    *PLEASE* take a look at the link I provided before starting for comments. Thank you in advance. – Your Common Sense Aug 18 '13 at 12:07