Trying to protect my SQL queries from injection attack and not having any luck

Question

I've read a couple of post's on here and followed the queries word for word (minus my variable names) and I cannot get my code to run on a website I'm working on without errors appearing. All I have is a form where user's can upload an image.

My original code which was vulnerable but worked:

<?php
  $uploadDir = 'images/';


  if(isset($_POST['upload']))
  {
    $fileName = $_FILES['userfile']['name'];
    $tmpName = $_FILES['userfile']['tmp_name'];
    $fileSize = $_FILES['userfile']['size'];
    $fileType = $_FILES['userfile']['type'];
    $memberID = $_POST['member-id'];
    $imgTitle = $_POST['img-title'];
    $catID = $_POST['catID'];

    $filePath = $uploadDir . $fileName;

    $result = move_uploaded_file($tmpName, $filePath);

    if (!$result) {
      echo "Error uploading file";
      exit;
     }

    echo "<br>Files uploaded<br>";

    if(mysqli_connect_errno())
     {
       printf("Connect failed: %s\n", mysqli_connect_error());
       exit();
     }

     if(!get_magic_quotes_gpc())
        {
         $fileName = addslashes($fileName);
         $filePath = addslashes($filePath);
        } 


       $query = "INSERT INTO `tblImage` (`fldImageID`, `fldMemberID`, `fldCatID`, `fldFilePath`, `fldName`) VALUES (NULL, '$memberID', '$catID', '$filePath', '$imgTitle')";

        $query = "SELECT `fldImageID` FROM `tblImage` ORDER BY `fldImageID` DESC LIMIT 1";


        $result = $conn->query($query) or die ("error");

 }

?>

and then the below is where I have tried to carry out prepared statements with no luck, please can someone point where my error is I've only pasted the below code which overwrites the above code where $query begins....

    $stmt = $conn->prepare = ("INSERT INTO tblImage (fldImageID, fldMemberID, fldCatID, fldFilePath, fldName) VALUES (NULL, ?, ?, ?, ?)");

    $stmt->bind_param($stmt, "ssss", $memberID, $catID, $filePath, $imgTitle); 
    mysqli_stmt_execute($stmt);
    $stmt->execute();
    $result = mysqli_stmt_get_result($stmt) or die ("error");

Error Message on web page:

You are executing twice. Remove `mysqli_stmt_execute($stmt);` — RiggsFolly, Mar 11 '19 at 19:15
It is so much easier if you stick to either Proceedural or the OO calling paradigm. If you mix you will ultimately get confused — RiggsFolly, Mar 11 '19 at 19:17
Also `$stmt = $conn->prepare =` should be `$stmt = $conn->prepare("....")` the `=` is not needed there — RiggsFolly, Mar 11 '19 at 19:17
Hi @RiggsFolly ah I hadn't realised I was mixing them, I seen a really highly voted answer on here and tried to follow that statement. Even after removing the execute duplication, doesn't seem to like it Cannot pass parameter 2 by reference — K.Haydock, Mar 11 '19 at 19:19

score 1 · Accepted Answer · answered Mar 11 '19 at 19:14

1

You have an extra value assigmnent:

$stmt = $conn->prepare = ("INSERT INTO tblImage (fldImageID, fldMemberID, fldCatID, fldFilePath, fldName) VALUES (NULL, ?, ?, ?, ?)");

just remove that operator:

$stmt = $conn->prepare("INSERT INTO tblImage (fldImageID, fldMemberID, fldCatID, fldFilePath, fldName) VALUES (NULL, ?, ?, ?, ?)");

answered Mar 11 '19 at 19:14

Lajos Arpad

45,912
26
82
148

Thank you @LajosArpad I completely missed that 2nd assignment! Although it still doesn't seem to like it.."Cannot pass parameter 2 by reference" – K.Haydock Mar 11 '19 at 19:25
Is it in the same line? – Lajos Arpad Mar 11 '19 at 19:28
1

@K.Haydock I think you should not pass $stmt, since this is object level call, where it's not needed. You should have something like: $stmt->bind_param("ssss", $memberID, $catID, $filePath, $imgTitle); – Lajos Arpad Mar 11 '19 at 19:31
That's it!! By the way you are all geniuses! Thanking you for all your time :-) – K.Haydock Mar 11 '19 at 19:32
One last thing, once the files successfully uploaded, the rest of the page no longer appears, do you know if I need a header redirect somewhere or? :) – K.Haydock Mar 11 '19 at 19:43
@K.Haydock is there an error message in your server log? – Lajos Arpad Mar 11 '19 at 19:43
@K.Haydock MAMP is either using Apache or Nginx. In either case, you should have some log files and if server error logging is enabled, then you will see the errors in the log files. – Lajos Arpad Mar 11 '19 at 19:50
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/189835/discussion-between-k-haydock-and-lajos-arpad). – K.Haydock Mar 11 '19 at 19:50

Trying to protect my SQL queries from injection attack and not having any luck

1 Answers1