Using PDO to avoid sql injection - new code doesn't work (newbie)

Question

I want to use prepared statements to avoid SQL injection.

To give you an idea, this was my old code:

<?php
    (connect to database = ok)

    $id = str_replace ('-', ' ', $_GET['id']);

    $sql = "SELECT * FROM `table-news` WHERE `id` = '$id' ORDER BY `date` DESC";
    $result = $conn->query($sql);
    if ($result->num_rows > 0) {
    while($row = $result->fetch_assoc()) {
?>

<div id="content">
    <?php echo strtolower($row['text']);?>
</div>

<?php
    }// end while
    }// end if
    else {
    echo '0 results';
    }// end else
?>

And this is the new code so far:

<?php
    $pdo = new PDO('mysql:host=localhost;dbname=testdb', 'root', '');               
    $id = str_replace ('-', ' ', $_GET['id']);                  
    $sql = "SELECT id, title, year, date, text FROM `table-news` WHERE id= :id ORDER BY `date` DESC";

    $stmt = $pdo->prepare($sql);
    $stmt->bindParam(":id", $id);
    $stmt->execute();

    if($result = $stmt->fetch(PDO::FETCH_ASSOC))
    {
?>

<div id="content">
    <?php echo strtolower($result['text']);?>
</div>

<?php
}// end if
else {
echo '0 results';
}// end else
?>

I got (at least) one problem with this new code:

The below code doesn't avoid SQL-injection. How can I transform this into a safe code using PDO ? (I really need to replace all spaces with a hyphen)

`$id = str_replace ('-', ' ', $_GET['id']);`

Your second snippet is binding `$_GET['id']` but you should be binding `$id` — , Jun 28 '14 at 10:05
IF you are using that id resulted from GET it doesnt matter,prepared statements will avoid sql injection. — Mihai, Jun 28 '14 at 10:07
@Stan I don't know now. You keep editing the question so it's not clear what your question is. — , Jun 28 '14 at 10:09
I only changed my code to bind $id like you suggested. I didn't edit the rest of the code so the question remains the same: is the code `$id = str_replace ('-', ' ', $_GET['id']);` good to avoid SQL-injection? Or do I need to transform that code too into something new using PDO ? — Stan, Jun 28 '14 at 10:57
You only have to worry about SQL Injection in the context of SQL itself; using PDO and prepared statements will make the query safe. The fact that you're doing a replacement first (which is _nowhere near enough_ on its own) is immaterial. PHP itself doesn't need to worry about the incoming parameter, except to hand it off in a safe manner, which is what prepared statements is for. — Clockwork-Muse, Jun 29 '14 at 11:37

score 1 · Answer 1 · answered Jun 28 '14 at 10:12

1

Replace this

$stmt->bindParam(":id", $_GET['id']);

with

$stmt->bindParam(":id", $id);

answered Jun 28 '14 at 10:12

Pramod K Singh

31
1

score 0 · Answer 2 · answered Jun 28 '14 at 23:41

Your code looks fine not sure why you think it does not avoid injections

Just get rid of the following line:

$id = str_replace ('-', ' ', $_GET['id']);

You don't need to manually clean the parameters. That is the whole point in preparing the query.

$id = $_GET['id'];      

$sql = "SELECT id, title, year, date, text FROM `table-news` 
        WHERE id= :id ORDER BY `date` DESC";

$stmt = $pdo->prepare($sql);
$stmt->bindParam(":id", $id);
$stmt->execute();

If you worry about the hypen then keep the line, but a single hypen is not as dangerous as the double hypens --like the SQL comments.

Matt The Ninja · Answer 3 · 2014-06-28T13:45:37.947

-2

To change the below to mysqli prepared see this...

  $sql = "SELECT * FROM `table-news` WHERE `id` = '$id' ORDER BY `date` DESC";
  $result = $conn->query($sql);

To..

//create db connection
  $conn = new mysqli("localhost", "my_user", "my_password", "world");

//create prepared statement
  $statement = $conn->prepare("SELECT * FROM `table-news` WHERE `id` = ? ORDER BY `date` DESC");

//bind parameters
  $statement->bind_param('s', $id);

//execute query
  $statement->execute();

Read up on these components further if you don't understand some bits. One bit they may look strange is the bind_param 's' value. This is telling MySQL that the param is a string. Let say you had two '?'s in your prepared statement first was a integer second was a string it would look like..

 $statement->bind_param('is', $MyInteger, $MyString);

edited Jun 28 '14 at 13:45

answered Jun 28 '14 at 10:14

Matt The Ninja

2,641
4
28
58

Escaping and parameterization? One suffices. – Gumbo Jun 28 '14 at 10:22
1

OP is using PDO. Why have you provided a `mysqli` example? – Jun 28 '14 at 10:31
'I want to use prepared statements to avoid SQL injection.' + '(newbie)'. `mysqli` will provide an easier-to-learn way of prepared statements. – Matt The Ninja Jun 28 '14 at 13:46
How is the mysqli API easier than PDOs? – Rangad Jun 28 '14 at 15:59
Because he is already using and understanding (in his old code) OOP MySQLi? – Matt The Ninja Jun 28 '14 at 16:43

Using PDO to avoid sql injection - new code doesn't work (newbie)

3 Answers3