0

So I have a function called "escape" that looks like this:

function escape($string){
    $escaped_string = mysqli_real_escape_string($this->conn, $string);
    return $escaped_string;
}

I before running a query I send a variable (originated from user input obviously) here so its escaped for security reasons.

Now I know its possible to use array_walk to apply an array of values to this function, but I just want to know if there is any reason why I shouldn't? I know it sounds like a daft question but it would be nice and easy to apply it to an array of user inputted values rather than each variable.

Normally if when making a function I will do it this way:

function whatever($user_input){
    $user_input = $this->escape($user_input);
    $this->query("SELECT dog from pets where owner = '$user_input'");
     e.c.t. 
}

But if I have a lot of user inputted data from a form for example id rather just pass an array into the function and use array_walk on the escape function to save myself the hassle. But again is there any particular reason (from a security point of view) why this is not a good idea?

Mike Abineri
  • 409
  • 2
  • 13
  • 4
    It *could* be argued that using `mysqli_real_escape_string()`, rather than creating a prepared statement with bound parameters, is bad (ok sub-optimal) practice... – CD001 Oct 31 '19 at 16:39
  • I know, but I dont know any PDO or anything about prepared statements and dont have the time or help to learn, most of what i make is for in house software. Not saying that means its safer it just means learning pdo isnt much of an option right now. – Mike Abineri Oct 31 '19 at 16:42
  • 2
    Your script is at risk for [SQL Injection Attacks](http://stackoverflow.com/questions/60174/). Use [prepared statements](https://bobby-tables.com/php). Even [escaping](http://stackoverflow.com/questions/5741187/) the string is not safe! You don't have to learn PDO. mysqli can do it also. – Jason K Oct 31 '19 at 16:42
  • 2
    A quick example of prepared statements is on https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php – Nigel Ren Oct 31 '19 at 16:46
  • I mean like I said our company doesnt have a HUGE number of employees, sure it will grow, but none of our in house systems are publicly accessable anyway, so the risk although it exists is pretty unlikely as the only people who know anything to do with technology at all in this company is our very small IT/Dev department. Escaping it is just a safety measure to easy to justify NOT doing. I will learn PDO at some point but for now I dont have that choice. – Mike Abineri Oct 31 '19 at 16:47
  • Ah well I didnt realise it was that simple @NigelRen thank you for the example. And thank you everyone for your advice! – Mike Abineri Oct 31 '19 at 16:49
  • 1
    @aynber - might have caused less confusion if you'd linked to [mysqli_prepare_statement](https://www.php.net/manual/en/mysqli.prepare.php) rather than the PDO version ;) – CD001 Oct 31 '19 at 16:50
  • 1
    @CD001 Ugh, copied the wrong line. That was silly of me. – aynber Oct 31 '19 at 16:50
  • Please be aware that the examples on php.net for prepared statements are unnecessarily complicated. It is much better to look at YCS's website for the correct examples. – Dharman Oct 31 '19 at 18:06

1 Answers1

3

YES, absolutely

The practice is the reincarnation of the infamous "magic quotes" feature, that once was a part of the language, but now thank goodness it is not.

Such an approach will do you no good but only a give a false feeling of security and spoil your data for no reason.

You must use prepared statements for all database interactions that involve PHP variables. This is the only 100% safe solution, and it makes the function in question obsolete.

Here I've got an example for the select query using prepared statements, https://phpdelusions.net/mysqli_examples/prepared_select

With a simple helper function it turns into much simpler and cleaner solution than that escaping-driven mess

Community
  • 1
  • 1
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • 3
    I love the way, in the comments, we're all trying to be *softly softly* and you just come in with a massive bulldozer of **OI! NOOO!** :D – CD001 Oct 31 '19 at 16:56
  • Is there a reason as to WHY mysqli_real_escape_string isn't safe? So far I can see lots of people saying its not safe but not specifying why. Would be good to know. – Mike Abineri Nov 01 '19 at 08:39
  • 2
    @MikeAbineri well this function is not meant to be "safe" in the first place. It has nothing to do with whatever safety, or security, or injections or anything like that. It's a humble **string formatting function** that mistakenly has been taken for a golden pill against SQL injections by generations of PHP programmers. Although it makes strings immune to injections, just as a side effect, with a bulk escaping like yours, you just cannot be sure whether it's a string or a number or a field name. Luckily, the tide has been turned, and the proper pill is prescribed now - parameterized queries. – Your Common Sense Nov 01 '19 at 08:50