How can I prevent sql injection in this class?

Question

I am trying to make a database class and want to have real_escape string in it but whenever I do so I don't get any results. Please help:

<!DOCTYPE html>
<head>
<title>PHP Classes</title>
</head>

<body>
<?php
class Database
{
    function Database($host, $user, $pass, $db){
        $this -> con = mysqli_connect($host, $user, $pass, $db);
    }
    function runQuery($query){
        mysqli_query($this -> con, $query);
    }
}
?>


<?php
    $database = new Database("localhost", "root", "", "website");
    $database -> runQuery("DELETE FROM subject WHERE name = 'Footer'");
?>
</body>
</html>

It's impossible to say from just this code. It depends entirely on how this code is used. If the program only passes hard coded queries to the `runQuery` method, then it's safe. If it builds SQL on the fly, it may well be vulnerable to SQL injection attacks. In other words, SQL injection is not a problem for a single function or class, it's a problem for the application as a whole. — p.s.w.g, Apr 07 '14 at 18:58
look at prepared statements and parameterized queries - [How can I prevent SQL injection in PHP?](http://stackoverflow.com/a/60496/689579) — Sean, Apr 07 '14 at 18:58
Why can't I use it in runQuery function before mysqli_query? — user3459778, Apr 07 '14 at 19:00
I would suggest you invest time in using prepared statements. It is also very unclear to me what value you expect to derive from this class. — Mike Brant, Apr 07 '14 at 19:03
Delete the question and check [this search result by Google](https://www.google.com.bd/search?q=php+sql+injection+prevention&rlz=1C1KMZB_enBD539BD539&oq=php+sql+in&aqs=chrome.3.69i57j0l5.6000j0j7&sourceid=chrome&espv=210&es_sm=122&ie=UTF-8), you'll learn more I think. — The Alpha, Apr 07 '14 at 19:08
possible duplicate of [How can I prevent SQL injection in PHP?](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) — Maks3w, Apr 07 '14 at 19:18
Why are you writing your own ORM? There are many out there already that are quite capable and field-proven. The burning irony here is you're writing an object-oriented wrapper around `mysqli` called in *procedural* mode. — tadman, Apr 07 '14 at 19:20

user229044 · Answer 1 · 2014-04-07T19:12:03.383

3

You can't, based on the code you've posted. Your class accepts an already assembled query, and at this point it's too late to perform any form of sanitization or escaping.

You need to protected from SQL-injection by escaping user-inputted data, before you assemble it into a query; ideally you would perform the final assembly via a parameterized query.

edited Apr 07 '14 at 19:12

answered Apr 07 '14 at 18:59

user229044

232,980
40
330
338

Just for clarification's sake, `mysqli` DOES support parameterized prepared statements (and actually provides better MySQL native support for them specific to MySQL than default PDO settings which would use emulated prepares for MySQL). As such, there is no reason to move to PDO from mysqli specifically to support paremetrized prepared statements. – Mike Brant Apr 07 '14 at 19:06
@MikeBrant, PDO can be configured to use non-emulated prepares too. And the usage is much easier than mysqli, especially for a general-purpose class like the one the OP is trying to write. – Bill Karwin Apr 07 '14 at 19:11
+1 to this answer by @meager. There is no protection possible inside the class, if users can submit arbitrary SQL statements. – Bill Karwin Apr 07 '14 at 19:12
1

@BillKarwin You are correct about ability to use native MySQL prepares via PDO, it just means you have a setting to change (which is why I noted emulated prepares are the default setting). My main point was to note that lack of prepared statements with parametrized parameter support is not a reason to select PDO over mysqli. That decision should reside in other factors (your personal preference in syntax style, future needs to support multiple database types, etc.). – Mike Brant Apr 07 '14 at 19:14

score 0 · Answer 2 · answered Apr 07 '14 at 19:38

I propose to you to use the singleton pattern for this class.

class Database
{
   private $database;   
   static $instance;

   private $query;
   private $result;


   private function __construct($params)
   {
       if($params!=null)
       {
           @$this->database = mysqli_connect($params->dbSettings['dbHostName'],$params->dbSettings['dbUserName'],$params->dbSettings['dbPassword']) or die('DB Error occured.');
           $this->query="";
           $this->result=null;
           mysqli_select_db($this->database,$class->dbSettings['dbName']);
       }    
       else
       {
           die('DB credentials not entered...');
            exit;   
       }
   }


   public static function GetInstance($params)
   {
       if(!(self::$instance instanceof self))
       {
          self::$instance = new self($params);  
       }            
       return self::$instance;
   }

   public function __destruct()
   {
       if($this->database)
       {
           mysqli_close($this->database);   
       }
   }

   public function ExecuteQuery($query)
   {                
        $this->query=addslashes($query);
        $this->result = mysqli_query($this->database,$query);
   }
   public function GetResult()
   {            
          if($this->result!=null)
          {
                 $returnArray=array();
                 $rows = mysqli_num_rows($this->result);
                 if($rows>0)
                 {                  
                        while($row = mysqli_fetch_assoc($this->result))
                        {
                               $returnArray[]=$row; 
                        }
                 }
                 return $returnArray;
          }
          else
          {
                 return null;
      }                      
   }            
}

Note that !!!. You must filter all data from user. Use for each field always the addslashes methos. Also the mysqli has some protection for sql injection.

How can I prevent sql injection in this class?

2 Answers2