2

How can I do this piece of code in a safe way to prevent SQL injections?

I tried to read the php manual of mysqli->prepared but I was not able to convert it since I'm new to PHP development.

NOTE: DAL::$conn is $msqli = new mysqli()

$objects = array();
        if($id != null)
        {
            $sql = "select * from Pages where id = ".$id;
        }
        else
        {
            $sql = "select * from Pages";
        }

        $result = mysqli_query(DAL::$conn, $sql);

        if (mysqli_num_rows($result) > 0) {
            // output data of each row
            $records = 0;

            while($row = mysqli_fetch_assoc($result)) {
                $records++;
                $data = new Pages();
                $data->id       =   $row["id"];
                $data->title    =   $row['title'];
                $data->content  =   $row["content"];
                $objects[$records] = $data;
            }
        } else {
            //No results
        }
E_net4
  • 27,810
  • 13
  • 101
  • 139
Stefan van de Laarschot
  • 2,154
  • 6
  • 30
  • 50

2 Answers2

5

Any query can be injected whether it's read or write, persistent or transient. Injections can be performed by ending one query and running a separate one (possible with mysqli), which renders the intended query irrelevant.

Any input to a query from an external source whether it is from users or even internal should be considered an argument to the query, and a parameter in the context of the query. Any parameter in a query needs to be parameterized. This leads to a properly parameterized query that you can create a prepared statement from and execute with arguments. For example:

SELECT col1 FROM t1 WHERE col2 = ?

? is a placeholder for a parameter. Using mysqli, you can create a prepared statement using prepare, bind a variable (argument) to a parameter using bind_param, and run the query with execute. You don't have to sanitize the argument at all (in fact it's detrimental to do so). mysqli does that for you. The full process would be:

$stmt = mysqli->prepare("SELECT col1 FROM t1 WHERE col2 = ?");
$stmt->bind_param("s", $col2_arg);
$stmt->execute();

There is also an important distinction between parameterized query and prepared statement. This statement, while prepared, is not parameterized and is thus vulnerable to injection:

$stmt = mysqli->prepare("INSERT INTO t1 VALUES ($_POST[user_input])");

To summarize:

  1. All Queries should be properly parameterized (unless they have no parameters)
  2. All arguments to a query should be treated as hostile as possible no matter their source
Drudge Rajen
  • 7,584
  • 4
  • 23
  • 43
  • Nice Thnx nice for the info. but what if the parameter can be null without a where ? – Stefan van de Laarschot Feb 13 '16 at 09:10
  • $stmt = mysqli->prepare("SELECT col1 FROM t1); – Stefan van de Laarschot Feb 13 '16 at 09:12
  • than $stmt->bind_param("s", $col2_arg); won't work right ? – Stefan van de Laarschot Feb 13 '16 at 09:12
  • 1
    @StefanvandeLaarschot i think you should look this manul http://php.net/manual/en/security.database.sql-injection.php – Drudge Rajen Feb 13 '16 at 09:16
  • "SQL injection works by tricking the script into including malicious strings when it creates SQL to send to the database." That means you don't need to prevent sql injection only for select queries. If you have parameters to be checked in the query i.e. in where condition of sql then you need to prevent sql injection . – Drudge Rajen Feb 13 '16 at 09:18
  • 1
    @StefanvandeLaarschot Also check the point one in the summarize section of answer "All Queries should be properly parameterized (unless they have no parameters)" if you have parameter in the query then you need to parameterized the query other wise you don't need. – Drudge Rajen Feb 13 '16 at 09:21
0

Thanks to @Drudge

this is my working Safe example :) Just to notice DAL = new $mysqli () its just my class. post it here so others can see it aswell :)

             function GetPages()
             {

                $objects = array ();

                $records = 0;
                $stmt = ($id == null ? DAL::$conn->prepare ( "select * from Pages" ) : DAL::$conn->prepare ( "select * from Pages where id = ?" ));
                if ($id != null) {
                    $stmt->bind_param ( "i", $id );
                }

                $stmt->execute ();

                $result = $stmt->get_result ();
                while ( $row = mysqli_fetch_assoc ( $result ) ) {
                    $records ++;
                    $data = new Pages ();
                    $data->id = $row ["id"];
                    $data->title = $row ['title'];
                    $data->content = $row ["content"];
                    $objects [$records] = $data;
                }

                return $objects;
          }

Thanks for the great tips

Stefan van de Laarschot
  • 2,154
  • 6
  • 30
  • 50