3
\$\begingroup\$

I'm building a checkout process where I am quite frequently making SQL connections based on user input so this is quite important. I want to know if it's well-protected from any SQL injection or other forms of SQL attack.

I'm currently trying to implement a prepared statement approach with some sanitizing with htmlspecialchars(),preg_match(), etc. Is this a safe function? Any help identifying what to add or change to make this function more secure would be appreciated.

Note: $arg is from user input

//Queries will generally be of this structure->"SELECT * FROM Users WHERE Id ?"

function fetchAssocPreparedStatements($query , $arg , $type) {

$servername = "xxx";
$username = "xxx";
$password = "xxx";
$dbname = "xxx";

$conn = mysqli_connect($servername, $username, $password, $dbname);
if ($conn->connect_error) {
    exit("An error occurred");
} 

     $arg = trim($arg); //try to sanatize
     $arg = stripslashes($arg);
     $arg = htmlspecialchars($arg);
     $arg = preg_replace("/[^a-z0-9\-]+/i", "", $arg); //now using preg_replace

     $stmt = mysqli_stmt_init($conn); //prepare and execute statement
     mysqli_stmt_prepare($stmt, $query);
     mysqli_stmt_bind_param($stmt, $type, $arg);
     mysqli_stmt_execute($stmt);


     $meta = $stmt->result_metadata(); //create assoc array with data
         while ($field = $meta->fetch_field()) { 
            $var = $field->name; 
            $$var = null; 
                $parameters[$field->name] = &$$var; 
         } 

     call_user_func_array(array($stmt, "bind_result"), $parameters);

     $copy = create_function('$a', 'return $a;');
     $results = array();

        while ($stmt->fetch()) {
            $results[] = array_map($copy, $parameters);
        }


     return $results; //returns results and closes the connections
     mysqli_stmt_close($stmt);
     mysqli_close($conn);
}
\$\endgroup\$
  • \$\begingroup\$ Titles should only state the code's purpose. Specific requests just stay in the post body. \$\endgroup\$ – Jamal Apr 17 '16 at 5:08
  • \$\begingroup\$ What the code is for is irrelevant, its more about whether or not the code itself is secure. I did not ask this question for any guidance on a checkout process, i asked this question for guidance on how to make my PHP sqli code more secure against sql injection (as my previous title stated)... i mean seriously, how is "Checkout process" a good representation of my question? (especially since the code provided is only querying a database and saving info, nothing really checkout related just standard stuff) \$\endgroup\$ – DiscreteTomatoes Apr 17 '16 at 5:14
  • 2
    \$\begingroup\$ Right, and like I said, the purpose stays in the title. We're not focusing on reviewing what's in the title; that's to tell us what the code is for. Your specific questions will still be read and likely and addressed. \$\endgroup\$ – Jamal Apr 17 '16 at 5:17
2
\$\begingroup\$

Since you are using bind to build your query all of these lines in your code really are not needed.

 $arg = trim($arg); //try to sanatize
 $arg = stripslashes($arg);
 $arg = htmlspecialchars($arg);
 $arg = preg_replace("/[^a-z0-9\-]+/i", "", $arg); //now using preg_replace

From the looks of what you posted you already know what you expect $arg to contain based on the fact that you are passing the bind type to the function ($type). You may use $type to check to make sure that the value of $arg is of the type you are expecting.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ yes this is sort of an old post and i removed all the sanitation since they are unnecessary with prepared statements (I Think). Thanks anyways! \$\endgroup\$ – DiscreteTomatoes Jul 18 '16 at 22:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.