PHP PDO not working, still able to sql inject

Question

Thanks for the help everyone, I'm closer. I ended up putting the _GET into the bind instead of setting vars, didn't see a point in that. I wasn't sure which answer to put in the SELECT, so this sample has the '?''. I tried both though. I also changed the bindParam to the sample above (:careerID and ;title). The good news is the injection won't work, but the bad news is I cannot click on the link to view the description. Here is the edited code, again I appreciate any assistance!

$conn = new PDO('mysql:host=XXXX;dbname=XXXX', 'XXXX', 'XXXX');
    // set the PDO error mode to exception
    $conn->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
    // Leave column names as returned by the database driver
    $conn->setAttribute(PDO::ATTR_CASE, PDO::CASE_NATURAL);
    // Convert Empty string to NULL
    $conn->setAttribute(PDO::ATTR_ORACLE_NULLS, PDO::NULL_EMPTY_STRING);   



$SQL = "SELECT *
          FROM careerapplicationpost,careerapplicationjobdescription 
         WHERE careerapplicationpost.CareerApplicationPostID = '?'
           AND careerapplicationjobdescription.JobDescriptionTitle = '?'";

$sth = $conn->prepare($SQL);
// binding parameters 
$sth->bindParam(':careerId', $_GET['CareerID'], PDO::PARAM_INT, 100);
$sth->bindParam(':title', $_GET['Title'], PDO::PARAM_STR, 100);
  // executing statement
$sth->execute();
$resultSet = $sth->fetchAll();
foreach ( $conn->query($SQL) as $row ) {
  
 
      
 //setup the postings
 echo "<h2>";
 echo "<a href=\"/careers/view-career.php?CareerID=$row[CareerApplicationPostID]&Title=$row[JobDescription]\">$row[JobDescriptionDisplayTitle]</a><br />"; 
 echo "</h2><hr />";
 echo "<br />";
 echo $row['Location'];
 echo ", &nbsp;&nbsp;";
 echo $row['FullTimePartTime'];
 echo  "<div class=\"postedon\">Posted on ";
 echo $row['PostedDate'];
 echo "</div>";
 echo "<br />";echo "<br />";
 echo "<strong>Summary:</strong>  ";
 echo $row['JobDescriptionSummary'];
 echo "<br />";echo "<br />";
 echo $row['JobDescriptionEdited'];
 echo "<div class=\"linebreak\">&nbsp;</div>";
 echo "<a href=\"/careers/files/DigiEmploymentApp.pdf\">Please fill out an application here.</a><br />";
 echo "<div class=\"clear\"></div>";
 echo "<hr />"; 
}

 

if (!$row['CareerApplicationPostID'])
{
 
header("Location:index.php");
 exit;
 }
$conn = null;

Possible duplicate of [How can I prevent SQL injection in PHP?](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) — user3942918, Aug 16 '18 at 05:12
PDO will not magically fix SQL injection. You need to use placeholders and bind the values, and not use the variables directly in the query. Replace `'$careerId'` and `'$title'` in the query with `?` (including the `'`). — Qirel, Aug 16 '18 at 05:14

Damian Dziaduch · Answer 1 · 2018-08-16T05:19:57.453

0

You can easily fix your code:

$SQL = "SELECT *
          FROM careerapplicationpost,careerapplicationjobdescription 
         WHERE careerapplicationpost.CareerApplicationPostID = :careerId
           AND careerapplicationjobdescription.JobDescriptionTitle = :title";

$sth = $conn->prepare($SQL);
// binding parameters 
$sth->bindParam(':careerId', $careerId, PDO::PARAM_INT);
$sth->bindParam(':title', $title, PDO::PARAM_STR, 100);

Problem was that you passed a query with values already passed in the string variable $SQL because you used variable inside of "".

edited Aug 16 '18 at 05:19

answered Aug 16 '18 at 05:17

Damian Dziaduch

2,107
1
15
16

Quoted placeholders will be treated as strings. – Qirel Aug 16 '18 at 05:18

Bibek Shah · Answer 2 · 2018-08-16T05:20:08.140

0

Change this:

 $SQL = "SELECT *
      FROM careerapplicationpost,careerapplicationjobdescription 
     WHERE careerapplicationpost.CareerApplicationPostID = '$careerId'
       AND careerapplicationjobdescription.JobDescriptionTitle = '$title'";

to this:

$SQL = "SELECT *
      FROM careerapplicationpost,careerapplicationjobdescription 
     WHERE careerapplicationpost.CareerApplicationPostID = :careerId
       AND careerapplicationjobdescription.JobDescriptionTitle = :title";

edited Aug 16 '18 at 05:20

answered Aug 16 '18 at 05:18

Bibek Shah

419
4
19

By giving it named placeholders, you must also update the bindings accordingly. – Qirel Aug 16 '18 at 05:19

score 0 · Answer 3 · answered Sep 05 '18 at 05:34

Just want to let anyone know visiting this page that the problem (besides the incorrect code) was a php version issue. When I was working on this code, I tried tons of examples and settled on the one above. After spending a few weeks learning PHP, I figured out I was running PHP 5.33 and after upgrading to 7, all of the samples that are posted everywhere worked. I ended up contacting my web hosting company and they upgraded PHP in a day, although if you are using a good web host, you should be able to upgrade it from your control panel. For those learning this, take your time and learn what the code does and what you will need out of it. PDO is a great tool to help you on your way to a bit more advanced PHP. I came here for a quick answer but I'm happy I didn't get one as now I understand the code since I had to learn it on my own.

PHP PDO not working, still able to sql inject

3 Answers3