PHP & MySQL: Preventing attacks - What's the different between these functions?

Question

I've been working off of some example code for logging into a database, and the author has provided two functions, for making entered data safe for the DB, with little explanation for why which is used where. I'm trying to figure out if it's a redundancy, and whether or not I shouldn't be using one of them at all, because it looks much... um... lighter in what it does.

Here's the first:

/*
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 = 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;
}

And now the second:

/* No explanation whatsoever... */
function SanitizeForSQL($str)
{
    if( function_exists( "mysqli_real_escape_string" ) )
    {
          $ret_str = mysqli_real_escape_string($connection, $str);
    }
    else
    {
          $ret_str = addslashes( $str );
    }
    return $ret_str;
}

The first code (which appears much more useful) looks like it's only used once when collecting the posted form fields into an array:

function CollectRegistrationSubmission(&$formvars)
{
    $formvars['Email'] = Sanitize($_POST['Email']);
}

And the second is used pretty much any time anything is placed into a table field in the database or pulled from the SESSION data, such as:

    $qry = "SELECT * FROM sessions WHERE Email='".SanitizeForSQL($Email)."'";

    /* or */

    $Email = SanitizeForSQL($_SESSION['email_of_user']);

My main concern is this seems redundant, but maybe that's because I don't understand it. Is there really a reason you would have to do both of these?

    $formvars['Email'] = Sanitize($_POST['Email']);

    $qry = "SELECT * FROM sessions WHERE Email='".SanitizeForSQL($formvars['Email'])."'";

So,

what's the difference?

Should I only be using one of these?

Should I be using something else entirely in one or both of these places?

Thank you for any light you can shed on this!

Don't use them, use a prepared statement. https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1 — AbraCadaver, Mar 12 '18 at 23:09
@AbraCadaver Not sure what you mean... can you clarify or point to a suitable post? — MateoMaui, Mar 12 '18 at 23:11
The code you show looks VERY old, there used to be lots of home-grown sanitizers. — AbraCadaver, Mar 12 '18 at 23:16
@AbraCadaver Doesn't surprise me in the least, everything else about the code seems ok, just this part makes me nervous. — MateoMaui, Mar 12 '18 at 23:18

score 0 · Answer 1 · answered Mar 12 '18 at 23:52

0

IMO rather than using specific database driver MySQLi, it's better to use Database Abstraction Layer. You can use PDO (PHP Data Objects).

<?php
/* Connect to a MySQL database using driver invocation */
$pdo = 'mysql:dbname=testdb;host=127.0.0.1';
$user = 'dbuser';
$password = 'dbpass';

try {
    $pdo = new PDO($dsn, $user, $password);
    $stmt = $pdo->prepare('SELECT * FROM sessions WHERE email = :email');

    // PDO will take care sanitizing the input so you do not need
    // to manually quote the parameters
    $stmt->execute([':email' => $_POST['email']]);

    $result = $stmt->fetchAll();
} catch (PDOException $e) {
    echo 'Connection failed: ' . $e->getMessage();
}

Further reading: http://php.net/manual/en/book.pdo.php

answered Mar 12 '18 at 23:52

rioastamal

75
6

If you're going to go that route, why not also adopt a more robust DBAL like Doctrine? http://www.doctrine-project.org/projects/dbal.html I mean, we can get real crazy on alternatives if we want to. – Anthony Mar 12 '18 at 23:57
@rioastamal Thank you for your response, I'm unfamiliar with PDO but it certainly looks more concise and something I should consider. I'll read up on it! – MateoMaui Mar 13 '18 at 02:56
@YourCommonSense Yeah, but it used only for simple example. – rioastamal Mar 13 '18 at 09:16
@MateoMaui, After familiar yourself with PDO, I recommend you to take a look also ORM library like Doctrine or Laravel Eloquent. – rioastamal Mar 13 '18 at 09:26
@rioastamal Thank you again for the guidance, I'm unfamiliar with everything being talked about here, starting with PDO. I've gathered that Doctrine or Eloquent is a layer over that... are there particular reasons I should choose one over the other? – MateoMaui Mar 13 '18 at 10:36
1

@YourCommonSense While I appreciate your sentiment, the honest truth is his example helped me partially because it's constructed similar to what I've already been looking at. I'm definitely reading your error handling referral to refine, but starting out this example was very good for me to get the basics. (Also, maybe I'm a special case... teaching myself HTML, JS, CSS, SQL, PHP, etc all over again after a head injury and a few years of mild vegetation, all to write a business survival tool for myself around concussion disorder. Keeping things simple and familiar is important at this point.) – MateoMaui Mar 13 '18 at 10:47
@rioastamal To clarify my previous question, I'm certainly good enough on PDO at this point... just not the DBAL layer over it that's now being recommended. I'm not sure I can tell the difference between why Doctrine or Eloquent would be a better option. Mahalo! (Also, I've already found this to get me started: https://stackoverflow.com/questions/1279613/what-is-an-orm-and-where-can-i-learn-more-about-it ) – MateoMaui Mar 13 '18 at 10:50
@YourCommonSense Again, I already said I'm reading your error reporting info to refine. Your point was taken. Already. – MateoMaui Mar 13 '18 at 11:05
@MateoMaui then I don't understand what was your comment about – Your Common Sense Mar 13 '18 at 11:05
@YourCommonSense Sometimes questions and the answers that are useful to the inquirer don't always need to conform to your expectations. You're implying that rioastamal's provided example is inherently and always bad... but for me, had he provided an example that satisfies your expectations of error handling, I may have not been able to make the transition from a previous concept so quickly because my concussed brain may have not recognized it. Some of us do need to start on a lower level, and I appreciated the example he gave me. – MateoMaui Mar 13 '18 at 11:19
@YourCommonSense Okaaaay.... "How can a simple example justify a wrong approach?" You asked that question of rioastamal, I just provided you the answer. I didn't misunderstand anything there. It's pretty clear in black and white a few comments up. – MateoMaui Mar 13 '18 at 11:40
@MateoMaui ok, you still don't get it but it doesn't worth an argument. As you wish. – Your Common Sense Mar 13 '18 at 11:47

PHP & MySQL: Preventing attacks - What's the different between these functions?

1 Answers1