php pagination security regarding INT

Question

I'm looking to update pagination on a page to PDO. However, I want to make sure it is 100% free from any SQL injection etc.

Below is the content of the pagination script I have found which I think will work without any issues. However as it will be pulling data from the URL I'm a bit concerned regarding the line:

if (isset($_GET["page"])) { $page  = $_GET["page"]; } else { $page=1; };

I can see isset is there to check if the variable is NULL (I think) but I can't see any checks for if it is not a number.

I was thinking of changing to:

if (isset($_GET["page"])) { $page  = (int)$_GET["page"]; } else { $page=1; };

As I think this will check if the page variable is a number. or should it be:

if (isset((int)$_GET["page"])) { $page  = $_GET["page"]; } else { $page=1; };

Or do I use INT on both? I think in old mysql you would have used striptags etc but not sure with PDO (still learning).

Here is the full code before the change mentioned above.

<?php
        include('connect.php');
        if (isset($_GET["page"])) { $page  = $_GET["page"]; } else { $page=1; };
        $start_from = ($page-1) * 3;        
        $result = $db->prepare("SELECT * FROM members ORDER BY id ASC LIMIT $start_from, 3");
        $result->execute();
        for($i=0; $row = $result->fetch(); $i++){
    ?>
    <tr class="record">
        <td><?php echo $row['a']; ?></td>
        <td><?php echo $row['b']; ?></td>
        <td><?php echo $row['c']; ?></td>
    </tr>
    <?php
        }
    ?>
</tbody>
</table>
<div id="pagination">
    <?php 

    $result = $db->prepare("SELECT COUNT(id) FROM members");
    $result->execute(); 
    $row = $result->fetch(); 
    $total_records = $row[0]; 
    $total_pages = ceil($total_records / 3); 

    for ($i=1; $i<=$total_pages; $i++) { 
                echo "<a href='index.php?page=".$i."'";
                if($page==$i)
                {
                echo "id=active";
                }
                echo ">";
                echo "".$i."</a> "; 
    }; 
    ?>

Connect.php contains

$db = new PDO('mysql:host='.$db_host.';dbname='.$db_database, $db_user, $db_pass);
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

Any help or guidance would be greatly appreciated. Also if you can spot anything else security wise, please let me know.

EDITS:

Added line:

$db->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);

to connect.php

Any other security tips?

score 0 · Accepted Answer · edited May 23 '17 at 12:23

0

Use parameterized queries this will avoid the possibility of injections. To do that using PDO you can

Pass the user provided values into the execute in an array
Place a question mark as a placeholder in place of the user data

$result = $db->prepare("SELECT * FROM members ORDER BY id ASC LIMIT ?, 3");
$result->execute(array((int)$start_from));

You also can bind it, http://php.net/manual/en/pdostatement.bindparam.php (based on doc and other threads, I don't usually bind).

$result = $db->prepare("SELECT * FROM members ORDER BY id ASC LIMIT :start, 3");
$result->bindParam(':start', (int)$start_from, PDO::PARAM_INT);
$result->execute();

Here's a longer thread on it, How can I prevent SQL injection in PHP?

Also PHP notes on it, http://php.net/manual/en/pdo.prepared-statements.php

For a longer sample, if you had multiple values coming in:

$result = $db->prepare("SELECT * FROM members where username = ? and email = ? ORDER BY id ASC LIMIT ?, 3");
$result->execute(array($_GET['name'], $_GET['email'], $start_from));

edited May 23 '17 at 12:23

Community

1
1

answered May 29 '15 at 15:59

chris85

23,846
7
34
51

Thanks, I'll read over the links provided. On a side note, changing to the above code killed the script, can't work out why (sorry still learning) – Phil May 29 '15 at 16:08
You changed it to the first version, `$result = $db->prepare("SELECT * FROM members ORDER BY id ASC LIMIT ?, 3"); $result->execute(array($start_from));` and it stopped working? Is it a 500, is there anything logged? – chris85 May 29 '15 at 16:12
Changed to: `$result = $db->prepare("SELECT * FROM members ORDER BY id ASC LIMIT ?, 3"); $result->execute(array($start_from)); ` Errors: AH01215: Stack trace: AH01215: PHP Fatal error: Uncaught exception 'PDOException' with message 'SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ''93', 3' at line 1' in index.php AH01215: #0 /index.php(64): PDOStatement->execute(Array) 64 is the line $result->execute(array($start_from)); – Phil May 29 '15 at 16:16
Wow guess you still have to cast it this way, never seen that before. – chris85 May 29 '15 at 16:28
any way to sanitize the GET other than using the above? (or do you know a pagination script (pdo) online that's safe? paid or free) – Phil May 29 '15 at 16:33
Yes, but the above method separates out the user data completely from the query. The sanitizing only attempts to remove bad data from the query and will probably miss something at some point. https://www.owasp.org/index.php/SQL_Injection_Prevention_Cheat_Sheet#Defense_Option_1:_Prepared_Statements_.28Parameterized_Queries.29 http://stackoverflow.com/questions/5130219/what-are-the-best-practices-to-prevent-sql-injections – chris85 May 29 '15 at 16:39

php pagination security regarding INT

1 Answers1

Linked