4

Possible Duplicate:
Are PDO prepared statements sufficient to prevent SQL injection?

my main concern with rolling out a new API Which I have been working on for a few days is the Security.

I'm a beginner to the PDO usage, but know the main structure. but I have no idea on how to protect the query from SQLInjection.

My code is Listed below:

<?php
    $Start = new Db();
    class Db 
    {
        private $dbh = null;

        public function __construct()
        {
            $this->dbh = new PDO('mysql:host=localhost;dbname=pdo', 'root', 'xxxxx');
        }

        public function PDOFetch($Var)
        {
            $sth = $this->dbh->prepare("$Var");
            $sth->execute();
            $result = $sth->fetchAll();
            return $result; 
        }

        public function PDONumb ($Var)
        {
            $sth = $this->dbh->prepare("$Var");
            $sth->execute();
            $count = $sth->rowCount();
            return $count;
        }

        public function PDODel ($Var)
        {
            $Delete = $this->dbh->exec("$Var");
            return $Delete;
        }

        public function PDOQuery ($Var)
        {
            $Query = $this->dbh->query("$Var");
            return $Query;
        }
    }
?>

How would I go About protecting from SQL Injection and other vulnerabilities?

Edit:

Queries Being passed into the API is being done from the "index.php" page for example.

A line would be:

    $Num = $Start->PDONumb("SELECT * FROM news");

But later, when I have covered my tracks with this. I want to go more advanced using this, so it will pass variables which user defines (hence the SQL injection question)

but at the moment, queries being passed through are defined by the administrator.

Community
  • 1
  • 1
Daryl Gill
  • 5,464
  • 9
  • 36
  • 69
  • 1
    Prevention against sql injection is indeed one of the points why people use PDO. It does offer protection. Why do you think you have to do that yourself? PDO is enough if you don't do the mistake to store additional sql statement tokens somewhere to combine these at execution time. – arkascha Dec 02 '12 at 15:51
  • @arkascha i'm new to using the api, i'm used to the mysql API which requires manual input for protection using pre-defined functions. So what your saying, that PDO protects it's self from Injection? – Daryl Gill Dec 02 '12 at 15:54
  • PDO does not protect itself, it protects your query if you use 'prepared statements'. You first prepare a statement with placeholders in it. That you bind values to those placeholders during execution. That is called 'binding'. Check Andy Lesters answer below. – arkascha Dec 02 '12 at 15:59
  • 1
    Please look at http://bobby-tables.com/php.html for examples on how to use placeholders. Also, you don't have to use PDO to use placeholder variables. – Andy Lester Dec 02 '12 at 16:02
  • 1
    PDO needs to have two things separated from each other: the statement to prepare and the paramters for its execution at runtime. DO NOT MIX THOSE. If you mix it by creating a string that holds a ready-to-execute sql statement there is nothing PDO can protect you from. You can wrap PDO, you always have to take two steps: First prepare a statement, have PDO bind the parameters to it. Don't bind them yourself using string functions. – arkascha Dec 02 '12 at 16:07

1 Answers1

4

We can't tell without seeing the SQL that you're passing in. If you're creating insecure SQL statements with untrusted data, then it doesn't matter if they get executed in PDO or not. You'll still be open to SQL injection.

For example, if you get $userid from the web and you build:

$sql = "SELECT * FROM users WHERE userid=$userid";

that SQL statement is open for SQL injection, because if the value of $userid is 0; DROP TABLE users;, then the SQL you will create will be

SELECT * FROM users WHERE userid=0; DROP TABLE users;

and it doesn't matter if you execute that through PDO or not: You're still executing code that a bad guy has sent you.

To use PDO properly, you need to bind your parameters.


Unrelated to your question, but an important point that novices often run into: It's unnecessary to put double quotes around your single variables. Your code

$Delete = $this->dbh->exec("$Var");

would be better written as

$Delete = $this->dbh->exec($Var);
Andy Lester
  • 91,102
  • 13
  • 100
  • 152
  • That is definitely true. I never even thought of the possibility that someone could use PDO that way. Good point to mention that! – arkascha Dec 02 '12 at 15:57
  • That's what it looks like OP's code is doing. Looks to me that it's just taking arbitrary SQL code from the caller and executing it. – Andy Lester Dec 02 '12 at 15:58
  • I'm basically producing this to minimize the entire process of performing PDO queries, reducing it to just 1 line and passing it into defined functions. See my edit of the post – Daryl Gill Dec 02 '12 at 16:00
  • 1
    @DarylGill : You are trying to abstract the abstraction layer. Don't. Using prepared statements the suggested PDO way is abstraction enough. – arkascha Dec 02 '12 at 16:01
  • 1
    @AndyLester you are right, that wrapper really prevents PDO to protect. I should look closer next time. Thanks for keeping the eyes open. – arkascha Dec 02 '12 at 16:03