1

After my prior post PHP Looping through elements and adding to Database

I went back to the drawing board, as I'm terrified of injection and would like some advice. Is the below "safe":

$stmt = $conn->prepare("INSERT INTO responses (skey, rtext) VALUES (?, ?)");
$stmt->bind_param("is", $skey, $rtext);

$skey = 1;
$rtext = mysqli_real_escape_string($conn,$_POST['Q1Answer']);
if(!$stmt->execute()){trigger_error("there was an error....".$con->error, E_USER_WARNING);}

$skey = 2;
$rtext = "Hello";
if(!$stmt->execute()){trigger_error("there was an error....".$con->error, E_USER_WARNING);}

$stmt->close();
$conn->close();

I could also call the below function:

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;
    }

Would any of the above help prevent injection?

Community
  • 1
  • 1
QS Train
  • 95
  • 2
  • 12

2 Answers2

2

When you bind variables to placeholder values your job is done. mysqli or PDO or whatever database driver you're using is responsible for safely encoding it from that point forward.

There is absolutely no need to add more sanitization functions on top of that, and many of these do more harm than good. The most important thing should be ensuring you never put unescaped data in your query, and the safest way to do that is to be extremely disciplined about using placeholder values.

If you pre-escape things then you'll have to un-escape them later. Data you assume is going to be used in HTML may very well turn up in a JSON document when you add an API to something, so now it has to be de-HTML-escaped before properly JSON escaping. This is really obnoxious.

Keep the data in your database as neutral as possible. That is, as raw as you can manage. If you're using Markdown, for example, keep the raw markdown in the database. If you're allowing certain HTML tags, put in whatever the user put in and scrub with a white-lister later. You may want to change your rules in the future to be more relaxed, and if you've already purged that content it's gone forever.

tadman
  • 208,517
  • 23
  • 234
  • 262
-2

You should use access token. For each database access you should define access token for client and without token query won't be executed. You can define your function like this:

function SanitizeForSQL($str,$token)
    {
      if($token=="Predefined token")
        {
         if( function_exists( "mysql_real_escape_string" ))
        {
              $ret_str = mysql_real_escape_string( $str );
        }
        else
        {
              $ret_str = addslashes( $str );
        }
        return $ret_str;
      }
        else return "Token is invalid".
    }
Akhter Al Amin
  • 852
  • 1
  • 11
  • 25
  • @Akhter - Thanks, why would I need to use a token? Is there a security reason (or just best practice)... – QS Train Apr 16 '17 at 05:04
  • I have no idea what `$token` is doing here, and I'm not sure it's a good idea. This is just doubling down on an already broken pattern. If it's an attempt at CSRF protection that token cannot be static, it must be cryptographically random and put into `$_SESSION`. – tadman Apr 16 '17 at 05:08
  • Yeah token must be random. Token will be created for each session created by Client. @tadman you got it right. – Akhter Al Amin Apr 16 '17 at 05:31
  • @QSTrain you should use it for standard procedure of authentication. – Akhter Al Amin Apr 16 '17 at 05:34