PHP string replace to prevent SQL injection?

Question

I've design this small function and I would like to know if anyone thinks it's safe enough, or if not, why.

    function safeSQLI($INPUT){
      // Trim un-needed spaces
      $safe_input = trim($INPUT);

      // Replace any SQL commands
      $safe_input = str_ireplace("drop",    "", $safe_input);
      etc...

      // Escape the result
      $safe_input = mysql_real_escape_string($safe_input);

      // Return the "Safe" result
      return $safe_input;
}

Answer: No, it's not safe at all. I am now using PDO and I think I was missing something great before now.

Safe maybe, but you are quite restricting the words a user can use. Nobody will ever be able to drop the bass again. — kero, Mar 07 '14 at 00:00
**Side note:** you can pass arrays into `str_ireplace`: http://php.net/str_ireplace — scrowler, Mar 07 '14 at 00:00
@user3390532 You should look at this question for possible security issues: http://stackoverflow.com/questions/6473823/php-mysql-real-escape-string-and-character — Anonymous, Mar 07 '14 at 00:03
or this... http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1 — scrowler, Mar 07 '14 at 00:03
[Why shouldn't I use mysql_* functions in PHP?](http://stackoverflow.com/questions/12859942/why-shouldnt-i-use-mysql-functions-in-php) - it's really sad how many times per day I post this link :( — Phil, Mar 07 '14 at 00:04
This question appears to be off-topic because it is asking for a codereview, which is what http://codereview.stackexchange.com is for. — Patrick Evans, Mar 07 '14 at 00:08
You know what's *even safer*? `$stmt->bind_param('s', $INPUT)` — Phil, Mar 07 '14 at 00:09
Beats me how ppl can upvote comment for this that starts with "safe maybe". This is absolutely unsafe. — Rid Iculous, Mar 07 '14 at 00:09
`mysql_stmt::bind_param()` must be called once, not once for each parameter. It's easier to use PDO, because then you can simply pass the $params array to `PDOStatement::execute()`. — Bill Karwin, Mar 07 '14 at 00:41
$stmt->bind_param('iss', $p1, $p2, $p3); ok so I would need to do something like this qith mysqli is there a lot of difference between PDO and MySQLi? — Mathieu, Mar 07 '14 at 00:48
Hi there. If you get an answer, please do not merge it into your question. New readers, such as myself, will have a rather confusing time working out what you mean by "Proof that it's not safe from: ntaso". — halfer, Mar 07 '14 at 00:49

score 3 · Accepted Answer · answered Mar 07 '14 at 00:06

3

str_ireplace() is generally a bad choice, because it doesn't work recursive. Try the following:

$safe_input = 'DELDELETEETE * FROM users';

Will result in:

DELETE * FROM users

So, your entire function falls back to mysql_real_escape_string() and everything that came before is useless. The point is: It's not impossible to write proper filtering methods, but it can be a real challenge to cover every single case there is.

You want to either follow a whitelisting approach and allow only certain types of content. This is tough to implement in the real world.

Or a blacklisting approach and deny certain characters. Most SQL injection vulnerabilites happen because one can inject additional commands in a string. If you escape the ' (or use mysql_real_escape_string(), you are usually safe). However, it depends on your web app if additional filtering is required or not.

Or use prepared statements.

answered Mar 07 '14 at 00:06

ntaso

614
6
12

2

`DROP * FROM users` is not a valid SQL query. ITYM `DELETE * FROM users`. – Barmar Mar 07 '14 at 00:09
Thanks, that's what I meant – ntaso Mar 07 '14 at 00:09
So I would need to make function that prepare my query and then for any query i would call those function passing it the inputs? – Mathieu Mar 07 '14 at 00:13
Read this, this is explains it pretty well: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1 – ntaso Mar 07 '14 at 00:15
Using MySQLi or PDO I just wanted to know if I can make it shorter to use. Cause there's is sometime a lot of input parameters and I don't want to write 10 line of code for a single query everytime i need to execute a query. I posted an exemple of this in the EDIT section of my initial question. – Mathieu Mar 07 '14 at 00:31

score 0 · Answer 2 · answered Mar 07 '14 at 00:02

0

It does prevent injection, provided you use quotes like you should:

SELECT * FROM `users` WHERE `name`='$username'

For example.

However it is complete overkill. mysql_real_escape_string is sufficient to make an input safe, provided you use the quotes as above.

I spent all day today trying to get some database work done on a server that had been locked down to return "Not Acceptable" HTTP responses if "anything that looked like a database table name was present in the request". Needless to say, it took significantly longer than it needed to, when a simple mysql_real_escape_string would have sufficed.

answered Mar 07 '14 at 00:02

Niet the Dark Absol

320,036
81
464
592

1

See [this question](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) about the "safety" of `mysql_real_escape_string` – kero Mar 07 '14 at 00:06
@kingkero Did you miss the part where I made the need for quotes clear in my answer? – Niet the Dark Absol Mar 07 '14 at 00:07
I think in the second answer of the posted link he uses quotes as well – kero Mar 07 '14 at 00:09
@kingkero Ye-e-e-es... under extremely contrived circumstances that should be avoided by setting the character set appropriately. – Niet the Dark Absol Mar 07 '14 at 00:12
Tbh I didn't fully read the linked post (neither did I downvote yours), thought it might be interesting in the context of safety – kero Mar 07 '14 at 00:14
Don't understand the -1 here, and it has not been justified, so +1. – halfer Mar 07 '14 at 00:50

score 0 · Answer 3 · answered Mar 07 '14 at 00:07

0

Not safe at all. Try running this:

 echo safeSQLI("drdropop tatableble TABLE_NAME");

answered Mar 07 '14 at 00:07

Rid Iculous

3,696
3
23
28

Yeah the result was: drop table _NAME – Mathieu Mar 07 '14 at 00:09
@user3390532 Obviously your table name isn't literally `TABLE_NAME` – Phil Mar 07 '14 at 00:11
you best option is to use an established library. if you want to write your own, you'd be best of to split the query into its parts and then access them separately. – Rid Iculous Mar 07 '14 at 00:14

score -2 · Answer 4 · answered Mar 07 '14 at 00:05

-2

That never let you insert a post about SQL in your blog.

I think use prepare statements and mysql_real_escape_string is safe enough.

PS: And you can avoid DDL sentences at BD level, with permissions.

answered Mar 07 '14 at 00:05

fmgonzalez

823
1
7
17

3

Use one or the other - not both. That's the kind of thinking what got OP in this mess in the first place. Use *correct* escaping, not *excessive* escaping. It's like people who use `htmlspecialchars` for inserting... – Niet the Dark Absol Mar 07 '14 at 00:06

PHP string replace to prevent SQL injection?

4 Answers4