0

I have a doubt about real escape string and security from sql injection..

Does have any difference if i put real escape string inside a query? like:

$ano = $_POST['ano'];
$SQL->query("DELETE from viewers WHERE ano = '".$SQL->real_escape_string($ano)."'");

or the correct method is doing like?

$ano = $SQL->real_escape_string($_POST['ano']);
$SQL->query("DELETE from viewers WHERE ano = '$ano'");

Ignore $SQL, its just a example.

Dharman
  • 30,962
  • 25
  • 85
  • 135
kanohn
  • 81
  • 5
  • 1
    I prefer outside, but in common, use prepared statements, you dont have to escape your values than like that. – Dorvalla Mar 01 '21 at 21:27
  • There is no functional difference between your two examples. Use whatever you feel is clearest. – Tangentially Perpendicular Mar 01 '21 at 21:42
  • You should do something like `$stmt = $SQL->prepare('DELETE from viewers WHERE ano=?'); $stmt->bind_param('s', $ano); $stmt->execute(); if($SQL->affected_rows){ /* there was at least one row affected */ } $stmt->close();` instead. – StackSlave Mar 01 '21 at 23:47
  • Where did you get that "$stmt->show_errors" part? there is no such function in mysqli. How to report errors in mysqli can be found in [this answer](https://stackoverflow.com/a/22662582/285587) – Your Common Sense Mar 02 '21 at 13:58

1 Answers1

2

Both approaches are incorrect. The solution is, instead, to use prepared statements with placeholder values.

There is no other safe way to deal with data parameters.

In other words this query should look like:

$stmt = $SQL->prepare('DELETE from viewers WHERE ano=?'); // Single quotes prevent accidental interpolation/injection
$stmt->bind_param('s', $_POST['ano']); // Just use $_POST directly
$stmt->execute();

This is the safe way to do it. There is zero chance of a SQL injection vulnerability.

There are only rare occasions where placeholder values will not work, but you need to address those on a case-by-case basis to see the best approach.

Your code with manual escaping should be ugly, and it should be blindingly obvious that all values are escaped, that there's absolutely no chance of the wrong variable being used due to a dumb typo. It should also include a comment in the form of an explanation and/or apology as to why a prepared statement could not work.

If use placeholder values consistently and diligently you won't have any SQL injection bugs. These are some of the worst to deal with because they often manifest in unusual circumstances that you might not normally test for, and can be exploited by attackers. Even a single SQL injection bug is enough to bust an application wide open, so the risk here is extremely high.

tadman
  • 208,517
  • 23
  • 234
  • 262
  • 1
    @TangentiallyPerpendicular I don't think you realize how endemic this problem is, nor how unique it is to the PHP community Tutorials *still* recommend dangerous approaches to this, decades after it was recognized to be a problem. – tadman Mar 01 '21 at 21:41
  • @tadman Yes, thank you. I have searched about prepare statements, however for my others queries, what var should i use? stmt = for first query. I can use any other var name? stmt2... etc. – kanohn Mar 01 '21 at 21:43
  • @tadman I realise it perfectly well, but bellowing it at all and sundry is a good way to alienate the very people you're trying to reach. – Tangentially Perpendicular Mar 01 '21 at 21:43
  • 1
    In general you should have simple conventions for this and stick to them. In most cases you only need one statement at a time, so as in the documentation, where `$stmt` is used, you can get away with just that. In cases where you're juggling two ore more statements you'll need to decide if you'll break with that convention and either use an array, or names like `$stmt_order_insert` to help organize things. Names like `$stmt2` doesn't really communicate what it is, so you might use it incorrectly without knowing. – tadman Mar 01 '21 at 21:45
  • @TangentiallyPerpendicular I've toned it down a bit but the frustration is real. – tadman Mar 01 '21 at 21:47
  • @tadman Someone told me to use functions for each query.. but i'm thinking how i will get the table values with these functions.. like getTableName() with returning stmt->getResult.. how i will get the table values? – kanohn Mar 01 '21 at 21:56
  • You can organize these calls into functions, that's not a bad idea, but it's also one step on the road to abstraction which usually concludes with using an ORM like [Eloquent](https://laravel.com/docs/8.x/eloquent). Each function will return a result set in whatever form you need, be that an array or an array of associative arrays and such. – tadman Mar 01 '21 at 22:03
  • 1
    @TangentiallyPerpendicular I cannot see the original comment but I can pretty much tell what it was. And I've got a very simple rule of thumb based on which you can determine whether it's worth to vent or not: as long as this notorious function is used for the purpose of preventing an SQL injection, it MUST be discarded in favor of prepared statements. – Your Common Sense Mar 02 '21 at 08:46
  • @YourCommonSense Clearly, you can't tell what the original comment was. I suggest you don't take issue with someone over what you think they said. – Tangentially Perpendicular Mar 02 '21 at 09:24