functions to prevent sql injection

Question

Suppose a developer does not use prepared statements in mysql, so are the following three functions enough to prevent any SQL injection. I would be grateful if someone improves the following functions:

function SanitizeForSQL($str)
{
    if( function_exists( "mysql_real_escape_string" ) )
    {
        $ret_str = mysql_real_escape_string( $str );
    }
    else
    {
        $ret_str = addslashes( $str );
    }
    return $ret_str;
}

/*
Sanitize() function removes any potential threat from the
data submitted. Prevents email injections or any other hacker attempts.
if $remove_nl is true, newline chracters are removed from the input.
*/
function Sanitize($str,$remove_nl=true)
{
    $str = $this->StripSlashes($str);

    if($remove_nl)
    {
        $injections = array('/(\n+)/i',
            '/(\r+)/i',
            '/(\t+)/i',
            '/(%0A+)/i',
            '/(%0D+)/i',
            '/(%08+)/i',
            '/(%09+)/i'
        );
        $str = preg_replace($injections,'',$str);
    }

    return $str;
}    
function StripSlashes($str)
{
    if(get_magic_quotes_gpc())
    {
        $str = stripslashes($str);
    }
    return $str;
}

Edit: Due to the response from the experts, it seems above code is a crap. So I will be using prepared statements. I thank all of you it was an eye opener.

`addslashes()` is to sql injection prevention as a piece of wet toilet paper is to cleaning up after a hurricane: it's utterly **USELESS**. your stripslashes function is USELESS and an outright syntax error - you cannot redeclare functions, especially built-in ones... unless this is all within an object that you haven't bothered showing. — Marc B, Jan 19 '14 at 09:14
@MarcB mysql_real_escape_string is always used addslashes() is in the else statements. — adnan kamili, Jan 19 '14 at 09:20
doesn't matter. if you end up using addslashes, you might as well **NOT** use it. it's utterly useless and utterly dangerous. You'll think you're safe and end up getting injected anyways. The entirety of this code is beyond pointless and useless. The mysql_*() functions are deprecated, and you should be using mysqli or PDO anyways, both of which have FAR BETTER injection prevention capabilities. Any system which still has magic_quotes enabled should be tortured to death, shot in the head, cut up into tiny pieces, burned to ashes, and the ashes scattered to the winds — Marc B, Jan 19 '14 at 09:22
If prepared statements are unavailable, properly use the escaping function provided by the database API instead. There's always one. If there isn't, use another API. — deceze, Jan 19 '14 at 09:23
@MarcB That's a pretty wishy-washy liberal stance you're taking there. — Matt Gibson, Jan 19 '14 at 09:31

Denis de Bernardy · Accepted Answer · 2014-01-19T09:35:16.440

The only way to improve your code is to scrap it altogether.

Besides being wrong in that you'll introduce charset-related bugs if you ever store multibyte strings and that your escaping function must be charset-aware, both the mysql extension and gpc quotes and deprecated and heading the way of the dodo. Don't use either.

Use pdo instead, or mysqli if you're married with mysql. In either case use parametrized queries -- also known as prepared statements.

And for that matter, you can also use an ORM that is well established and thoroughly battle-tested. There are many. This ORM will take care of the SQL for you, so you can worry about actually writing your app instead of your own framework.

functions to prevent sql injection

1 Answers1