Is this a safe way to prevent database injection?

Question

Often times I use methods like this to filter input. Using preg_match( ) as shown, or sometimes using a switch( ) that only acts if a POST/GET variable is one of a specific number of keywords.

if (preg_match("/^[0-9]+$/",$_POST['id_to_clear']))
    {
    db_query("UPDATE `table` SET `error`=0, `progress`=0 WHERE `id` = {$_POST['id']}");
    }

Generally speaking, using a POST/GET variable directly in an SQL query is a huge red flag. But in this case it seems perfectly safe. Is there any way this could go awry? Something I'm not seeing?

use prepared statements: https://dev.mysql.com/doc/refman/8.0/en/sql-syntax-prepared-statements.html — Mitch Wheat, Dec 24 '18 at 08:30
"Some people, when confronted with a problem, think 'I know, I'll use regular expressions.' Now they have two problems." — [Jamie Zawinsky](https://en.wikiquote.org/wiki/Jamie_Zawinski). It's general wisdom that query parameters are both safer to prevent SQL injection, and much easier to code than regular expression filtering solutions like the one you show. — Bill Karwin, Dec 25 '18 at 19:45

score 2 · Answer 1 · answered Dec 24 '18 at 08:32

2

You should probably just use prepared statements here, and PHP offers a number of APIs which implement this. That being said, if the following regex passes:

preg_match("/^[0-9]+$/", $_POST['id_to_clear'])

Then the id_to_clear POST value should contain only digits, in which case I don't see any way which the query you showed us could be injected.

But beware, because even though things might be safe now, in the future, your query might have a problem. Without too much difficulty, I could imagine a refactor later on which again exposes the query to SQL injection. So, stick with statements, unless there is a compelling reason to do otherwise.

answered Dec 24 '18 at 08:32

Tim Biegeleisen

502,043
27
286
360

Weird part is validate `$_POST['id_to_clear']` but then use `$_POST['id']` :/ – Juan Carlos Oropeza Dec 24 '18 at 08:35
1

@JuanCarlosOropeza That's gotta be a typo :P – Tim Biegeleisen Dec 24 '18 at 08:35
Yes that was a typo, this isn't my exact code, just a very representative sample. – l008com Dec 24 '18 at 08:47
@l008com a "representative sample" still needs to be actual working code. Posting something you have not verified is wasting the time of volunteers, and not consistent with how it should be done. See https://stackoverflow.com/help/mcve. – Michael - sqlbot Dec 24 '18 at 13:02
@TimBiegeleisen Can you give me an example of how this might not be safe later on? Because I can't think of any :/ – l008com Dec 26 '18 at 08:40
Well someone else could inherit and refactor your code without sterilizing the input. If you use a statement that person will probably continue to do so. – Tim Biegeleisen Dec 26 '18 at 08:43

Is this a safe way to prevent database injection?

1 Answers1