HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Checkout process

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checkoutprocessstackoverflow

Problem

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

Solution

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.

Code Snippets

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

Context

StackExchange Code Review Q#125829, answer score: 2

Revisions (0)

No revisions yet.