4

I've written a bit of code below. I wanted to know two things.

1. Does my code have any vulnerabilities?

2. Do I need to use mysqli_real_escape_string with prepared statements? Would it act as an extra layer of security or is it redundant?

Code Used:

if ($_SERVER['REQUEST_METHOD'] == 'POST') 
{
$admin_type = mysqli_real_escape_string($conn, $_POST['admin_type']);
$position = mysqli_real_escape_string($conn, $_POST['position']);
$first_name = mysqli_real_escape_string($conn, $_POST['first_name']);
$last_name = mysqli_real_escape_string($conn, $_POST['last_name']);
$user_name = mysqli_real_escape_string($conn, $_POST['user_name']);
$email = mysqli_real_escape_string($conn, $_POST['email']);
$phone_number = mysqli_real_escape_string($conn, $_POST['phone_number']);
$passwd = mysqli_real_escape_string($conn, $_POST['passwd']);
$created_at = mysqli_real_escape_string($conn, $_POST['created_at']);
$about = mysqli_real_escape_string($conn, $_POST['about']);

$sql = "INSERT INTO admin_accounts (admin_type, position, first_name, last_name, user_name, email, phone_number, passwd, about) VALUES (?,?,?,?,?,?,?,?,?);";

$stmt = mysqli_stmt_init($conn);
if (!mysqli_stmt_prepare($stmt, $sql)) {
    echo "SQL Error";
} else {
    mysqli_stmt_bind_param($stmt, "sssssssss", $admin_type, $position, $first_name, $last_name, $user_name, $email, $phone_number, $passwd, $about);
    mysqli_stmt_execute($stmt);
  {
     $_SESSION['success'] = "Admin user added successfully!";
     header('location: admin_users');
     exit();
    }     
}
    
}
Bill Karwin
  • 499,602
  • 82
  • 638
  • 795
  • Your code still leaves a door open for those sniffing out plain text passwords and are relatively easy to find/figure out. I'd look into hashing and storing passwords properly if I were you, given if you plan on going online with this. Those functions are called `password_hash()` and to verify them is called `password_verify()`. Look those up on the web; you'll get a lot of (good) hits including the manuals on php.net. Btw, I closed the question with which you can consult. – Funk Forty Niner Aug 10 '18 at 01:18
  • of course, i was actually doing that right now! Thank you :) –  Aug 10 '18 at 02:07
  • you mean the functions I posted in my comment earlier? – Funk Forty Niner Aug 10 '18 at 02:14
  • Yes, i was reading into it! –  Aug 10 '18 at 02:20

2 Answers2

2

An age old argument both PHP Prepared Statement and PHP mysqli_real_escape_string do not perform the same function although both share the same goal of trying to get your string or user input in a format that may not cause harm to your database i used the term may because lack of proper validation will still result to same problems.

take for instance a delete query for users to delete their own post without proper validation user may delete posts of other users and user wont even need an SQL injection to archive that.

string mysqli_real_escape_string ( mysqli $link , string $escapestr )

This function is used to create a legal SQL string that you can use in an SQL statement. The given string is encoded to an escaped SQL string, taking into account the current character set of the connection now this means that mysqli_real_escape_string is dependent on your current character set.

while prepared statements binds your values / user inputs to a specific Data type and instructs your sql to treat it a such but this is also dependent on you specifying the data type.

basically mysqli_real_escape_string will take out the junk in the user input according to a defined character set while prepared statements will make sure if there be any junk in the user input it will be only interpreted as you want it to be interpreted which is usually as user junk :)

what am try to say is using both is not a bad idea but protection of your data base falls to you the developer to take all necessary and logical step to archive maximum security no one cares how fast the hacked site was!!

Bobby Axe
  • 1,594
  • 10
  • 25
1

Escaping is redundant with respect to using query parameters. Don't use escaping if you are also using bound query parameters. You'll end up with data in your database that has literal backslashes in it.

Just use query parameters. Binding parameters is easier and simpler than all those calls to mysqli_real_escape_string().

What's even easier is using PDO, because you don't even need to do binding.

Example:

if ($_SERVER['REQUEST_METHOD'] == 'POST')
{   
    $param_keys = ['admin_type'=>null, 'position'=>null, 'first_name'=>null, 
        'last_name'=>null, 'user_name'=>null, 'email'=>null, 'phone_number'=>null, 
        'passwd'=>null, 'created_at'=>null, 'about'=>null];

    // make sure the params include all keys in $param_keys,
    // but no keys that aren't in $param_keys
    $params = array_merge($param_keys, array_intersect_key($_POST, $param_keys));

    $sql = "
        INSERT INTO admin_accounts 
        SET admin_type = :admin_type,
            position = :position,
            first_name = :first_name,
            last_name = :last_name,
            user_name = :user_name,
            email = :email,
            phone_number = :phone_number,
            passwd = :passwd,
            about = :about";

    $stmt = $pdoConn->prepare($sql);
    $stmt->execute($params);
    $_SESSION['success'] = "Admin user added successfully!";
    header('location: admin_users');
}   

I'm assuming you configured PDO to throw exceptions on error.

Bill Karwin
  • 499,602
  • 82
  • 638
  • 795
  • Perfect Thank you! Ill be waiting for that :) –  Aug 10 '18 at 00:40
  • Undefined variable: pdoConn in C:\xampp\htdocs\test\add_admin.php on line 36 –  Aug 10 '18 at 00:45
  • I seem to be getting the above error –  Aug 10 '18 at 00:45
  • Btw I have not configured PDO to throw exceptions on error. I've actually never used PDO before –  Aug 10 '18 at 00:46
  • The variable `$pdoConn` is meant to be a connection you created with [new PDO(...)](http://php.net/manual/en/pdo.construct.php), instead of `new mysqli(...)` – Bill Karwin Aug 10 '18 at 00:47
  • You're welcome to continue using mysqli if you want, but I recommend using PDO instead. Mysqli was introduced in PHP 5.0, and PDO was introduced in PHP 5.1. Yet some people still believe that PDO "might not be there," despite it being a standard part of PHP for more than 10 years. – Bill Karwin Aug 10 '18 at 00:49
  • Try this PDO tutorial, it's very good: https://phpdelusions.net/pdo – Bill Karwin Aug 10 '18 at 00:50
  • 1
    Thank you so much, I appreciate your help! –  Aug 10 '18 at 00:55