0

Context: I'm trying to convince a friend to switch to using parameterized queries to prevent SQL injections and other malicious attempts as that is the standards these days but he has a mentality of "If it's not broken, don't fix it."

Here's the code he currently uses:

function sql_safe($text) {
    return str_replace("'", "''", $text);
}

Is there a way for me to break this function to illustrate to him that this approach is not advisable any more?

Additional Info

It's being used as a general means to protect the system from SQL injections so that user inputs are escaped properly. But I feel like his approach could break at certain scenarios which I haven't figured out yet.

halfer
  • 19,824
  • 17
  • 99
  • 186
dokgu
  • 4,957
  • 3
  • 39
  • 77
  • Can you add a bit more context how this is intended to be used please? – πάντα ῥεῖ Jan 16 '17 at 21:03
  • @πάνταῥεῖ It's being used as a general means to protect the system from SQL injections so that user inputs are escaped properly. But I feel like his approach could break at certain scenarios which I haven't figured out yet. – dokgu Jan 16 '17 at 21:05
  • [Add such additional information to your question](http://codereview.stackexchange.com/posts/152809/edit) please, rather than in a comment. – πάντα ῥεῖ Jan 16 '17 at 21:22

1 Answers1

4

Here's your code:

<?php
function sql_safe($text) {
    return str_replace("'", "''", $text);
}
echo "SELECT * FROM db WHERE field = '" . sql_safe($argv[1]) . "';\n";

And here's the most obvious way of breaking it:

$ php ./x.php "\' OR TRUE; -- MySQL"
SELECT * FROM db WHERE field = '\'' OR TRUE; -- MySQL';

Stack Overflow has covered the topic of SQL injection extensively over the years. See for example Can I protect against SQL Injection by escaping single-quote and surrounding user input with single-quotes? . There's a neat trick in there that exploits "maximum length of string" to truncate just one of the replacement ''s.

Community
  • 1
  • 1
Quuxplusone
  • 23,928
  • 8
  • 94
  • 159