1
\$\begingroup\$

I just heard of prepared statement in PHP and decided to prevent SQL injection with it and wrote a script testing it.

I would like to know what security threats this script can prevent, what security threats this script is vulnerable to, and how these threats are blocked.

<?php
require_once ("db.php");
$db = new MyDB();
session_start();

if (isset($_POST['userMsgField']) && !empty($_POST['userMsgField']) || isset($_POST['hash']) && !empty($_POST['hash']))
{
$my_id = $_SESSION['log_id'];
$rep_msg = $_POST['userMsgField'];
$hash = $_SESSION['hash'];
$flag = 0;

$sql =<<<EOF
SELECT * FROM connect WHERE (user_one = '$my_id' AND hash = '$hash') OR (user_two = '$my_id' AND hash = '$hash');
EOF;

$ret = $db->query($sql);

while ($row = $ret->fetchArray(SQLITE3_ASSOC))
{
    $user_one = $row['user_one'];
    $user_two = $row['user_two'];

    if ($user_one == $my_id)
    {
        $to_id = $user_two;
    }
    else
    {
        $to_id = $user_one;
    }


    $isql =<<<EOF
    INSERT INTO messager (message, group_hash, from_id, flag, to_id) VALUES (:message, :group_hash, :from_id, :flag, :to_id);
EOF;
    $bsql =<<<EOF
    INSERT INTO chatportal (message, group_hash, from_id, flag, to_id) 
VALUES (:message, :group_hash, :from_id, :flag, :to_id);
EOF;

    $stmt = $db->prepare($isql);
    $bstmt = $db->prepare($bsql);

    $stmt->bindValue(':message', $rep_msg, SQLITE3_TEXT);
    $stmt->bindValue(':group_hash', $hash, SQLITE3_INTEGER);
    $stmt->bindValue(':from_id', $my_id, SQLITE3_INTEGER);
    $stmt->bindValue(':flag', $flag, SQLITE3_INTEGER);
    $stmt->bindValue(':to_id', $to_id, SQLITE3_TEXT);

    $bstmt->bindValue(':message', $rep_msg, SQLITE3_TEXT);
    $bstmt->bindValue(':group_hash', $hash, SQLITE3_INTEGER);
    $bstmt->bindValue(':from_id', $my_id, SQLITE3_INTEGER);
    $bstmt->bindValue(':flag', $flag, SQLITE3_INTEGER);
    $bstmt->bindValue(':to_id', $to_id, SQLITE3_TEXT);

    $result = $stmt->execute();
    $bresult = $bstmt->execute();

    if ($reuslt && $bresult)
    {

    }
}
}
\$\endgroup\$
  • \$\begingroup\$ Please first convert the first SQL statement to a prepared statement. And fix the typo in a variable name. \$\endgroup\$ – Roland Illig Jul 4 '17 at 21:28
  • \$\begingroup\$ You should read about Little Bobby Tables if you don't know what SQL injection is. \$\endgroup\$ – I wrestled a bear once. Jul 5 '17 at 12:51
3
\$\begingroup\$

Non-review answers

  1. what security threats this script can prevent - sql injection
  2. what security threats this script is vulnerable to - sql injection
  3. how these threats are blocked - https://stackoverflow.com/questions/8263371/how-can-prepared-statements-protect-from-sql-injection-attacks

Review answer.

Your code is bloated with unnecessary operators, such as heredoc. It is also recommended to use PDO instead of sqlite, as it will let you to use array binding, which is especially useful in this case as you are inserting equal sets of data.

Besides, you should use prepared statements for your database interactions.

There are also other improvements and fixes.

if (!empty($_POST['userMsgField']) && !empty($_POST['hash']))
{
    $my_id = $_SESSION['log_id'];
    $rep_msg = $_POST['userMsgField'];
    $hash = $_POST['hash'];
    $flag = 0;

    $sql = "SELECT * FROM connect WHERE (user_one = ? AND hash = ?) 
                                     OR (user_two = ? AND hash = ?)";
    $stmt = $db->prepare($sql);
    $stmt->execute([$my_id,$hash,$my_id,$hash]);

    while ($row = $stmt->fetch(PDO::FETCH_ASSOC))
    {
        $user_one = $row['user_one'];
        $user_two = $row['user_two'];

        $to_id = ($user_one == $my_id) ? $user_two : $user_one;

        $data = [
            'message' => $rep_msg,
            'group_hash' => $hash,
            'from_id' => $my_id,
            'flag' => $flag,
            'to_id' => $to_id,
        ];

        $sql ="INSERT INTO messager (message, group_hash, from_id, flag, to_id) 
                            VALUES (:message,:group_hash,:from_id,:flag,:to_id)";
        $db->prepare($sql)->execute($data);

        $sql ="INSERT INTO chatportal (message, group_hash, from_id, flag, to_id)
                              VALUES (:message,:group_hash,:from_id,:flag,:to_id)";
        $db->prepare($sql)->execute($data);
    }
}

Now this code is much cleaner and easier to read. And safe, of course.

\$\endgroup\$

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.