1

When I was learning about SQL security and preventing SQL injection, I learnt that its better to use bindparam when fetching results for an id like this:

//Prepare Statement
$stmt = $mysqli->prepare("SELECT * FROM my_table WHERE id=?");

if ( false===$stmt ) {
  die('prepare() failed: ' . htmlspecialchars($mysqli->error));
}

$rc = $stmt->bind_param("i", $id);

if ( false===$rc ) {
  die('bind_param() failed: ' . htmlspecialchars($stmt->error));
}

$rc = $stmt->execute();

if ( false===$rc ) {
  die('execute() failed: ' . htmlspecialchars($stmt->error));
}


// Get the data result from the query. You need to bind results for each column that is called in the prepare statement above
$stmt->bind_result($col1, $col2, $col3, $col4);
 
/* fetch values and store them to each variables */
while ($stmt->fetch()) {
   $id = $col1;
   $abc = $col2; 
   $def = $col3; 
   $xyz = $col4;
}

$stmt->close();
 
$mysqli->close();

Atm, when I am fetching all results, I am using this:

$query= "SELECT * FROM my_table";  
$result=mysqli_query($connect, $query);

if (!$result)  
{  
  die('Error fetching results: ' . mysqli_error()); 
  exit();  
}

echo '<table border="1">'; // start a table tag in the HTML
//Storing the results in an Array
while ($row = mysqli_fetch_array($result))  //Creates a loop to loop through results
{  
    echo "<tr><td>" . $row['abc'] . "</td><td>" . $row['def'] . "</td><td>" . $row['xyz'] . "</td></tr>";  
}  
echo '</table>'; //Close the table in HTML

My question is:

  1. For my second code, do I need to use bind_result when fetching all results for any security reasons similar to my first example?

  2. If yes, how can I use prepare statement with bind_result when I am fetching all results and not using $id?

  3. If I use the second example the way it is for fetching all results, are there any security issues?

Dharman
  • 30,962
  • 25
  • 85
  • 135
Neel
  • 9,352
  • 23
  • 87
  • 128
  • Sidenote: You have this the other way around `if ( false===$stmt ) {` that should read as `if($stmt===false){` least... am pretty sure, unless there's something I don't know (yet). – Funk Forty Niner Jan 02 '14 at 17:37
  • 1
    If you're not interacting with the user and all the input it by you than you don't have to bind_result. Bind result is only used when you have variables in the query itself. I don't see any issues with example 2, provided the query does not have any user interaction – Piotr Kaluza Jan 02 '14 at 17:37
  • @-Fred -ii it's actually correct 1 === 1 either way. The way he worded it is actually good practice for to avoid null – Piotr Kaluza Jan 02 '14 at 17:41
  • @PiotrKaluza this is wrong wording. Interacting with user has nothing to do with prepared statements. – Your Common Sense Jan 02 '14 at 17:42
  • what i meant is having placeholders within query, that would be one obvious case of using a prepared statement. Others would be performance, and convenience. I should have been more clear. – Piotr Kaluza Jan 02 '14 at 17:47
  • @Fred -ii: I adapted the code I found here: `http://stackoverflow.com/questions/2552545/mysqli-prepared-statements-error-reporting` But having it as `$stmt===false` may make it easy when reading the code I guess. @Piotr Kaluza: Thank you for your confirmation. Yes, there wont be any user input and its only going to be a simple view all results query. Thats what I was wondering too if there was a need to use bind_results when there is no _POST data used like `$id`, etc.. Thank you so much for clarifying this. – Neel Jan 02 '14 at 17:55
  • Hm... interesting. Thanks for the info, it's definitely noted. @blackops_programmer (I stand corrected) ;-) Cheers – Funk Forty Niner Jan 02 '14 at 18:02
  • Thanks @PiotrKaluza The OP gave me the reference link. I for one did not know that till now. Cheers – Funk Forty Niner Jan 02 '14 at 18:07
  • @PiotrKaluza & @fred-ii `false==$stmt` is better than `$stmt==false` not because of nulls or readability, but because it causes an error when you accidentally assign instead of compare, `$stmt=false` will work just fine, but you get an compile or runtime error later in your program if you're lucky, or your program simply returns wrong results. When you mistype so it says `false=$stmt` you will immediately get an error message right where you made the mistake. – Rinze Smits Jan 08 '14 at 07:49

1 Answers1

5

1) For my second code, do I need to use bind_result when fetching all results for any security reasons similar to my first example?

No. bind_result() has nothing to do with security. You can use whatever method you wish with any query.

2) If yes, how can I use prepare statement with bind_result when I am fetching all results and not using $id?

Exactly the same way as with any other query. There is no difference actually. and having any particular variable doesn't matter at all.

3) If I use the second example the way it is for fetching all results, are there any security issues?

There is always a security issue. But none from the area of SQL injection in this snippet. You may wish to check for XSS issues.

Just to clarify your ambiguous question:
In case you are confusing bind_result with bind_param, here is a rule of thumb: you have to use a placeholder (and thus bind_param()) for the every variable that is going into query. Always. No exceptions.

From this rule you can simply tell if you need to use prepare() or not in any particular case.

Also, there is no need for such a long and windy code in the first example.

$stmt = $mysqli->prepare("SELECT * FROM my_table WHERE id=?");
$rc = $stmt->bind_param("i", $id);
$rc = $stmt->execute();
$stmt->bind_result($id, $abc, $def, $xyz);
while ($stmt->fetch()) {
  echo $id;
}

Just set mysqli in exception mode before connect:

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345