-1

I want to make this page secure, but i dont know where should i start. Because i've got injected yesterday I treid mysql escaping string, but i didn't help much. And i dont know anything about PDO, can you hook me up? Here's the code.

<?php
//category.php

require_once('startsession.php');
require_once('php/mysql_prisijungimas.php');
include 'connect.php';

//first select the category based on $_GET['cat_id']
$sql = "SELECT
            cat_id,
            cat_name,
            cat_description
        FROM
            categories
        WHERE
            cat_id = " . mysql_real_escape_string($dbc, trim($_GET['id']));

$result = mysql_query($sql);

if(!$result)
{
    echo 'The category could not be displayed, please try again later.' . mysql_error();
}
else
{
    if(mysql_num_rows($result) == 0)
    {
        echo 'This category does not exist.';
    }
    else
    {
        //display category data
        while($row = mysql_fetch_assoc($result))
        {
            echo '<h2>Topics in &prime;' . $row['cat_name'] . '&prime; category</h2><br />';
        }

        //do a query for the topics
        $sql = "SELECT  
                    topic_id,
                    topic_subject,
                    topic_date,
                    topic_cat
                FROM
                    topics
                WHERE
                    topic_cat = " . mysql_real_escape_string($dbc, trim($_GET['id']));

        $result = mysql_query($sql);

        if(!$result)
        {
            echo 'The topics could not be displayed, please try again later.';
        }
        else
        {
            if(mysql_num_rows($result) == 0)
            {
                echo 'There are no topics in this category yet.';
            }
            else
            {
                //prepare the table
                echo '<table border="1">
                      <tr>
                        <th>Topic</th>
                        <th>Created at</th>
                      </tr>';   

                while($row = mysql_fetch_assoc($result))
                {               
                    echo '<tr>';
                        echo '<td class="leftpart">';
                            echo '<h3><a href="topic.php?id=' . $row['topic_id'] . '">' . $row['topic_subject'] . '</a><br /><h3>';
                        echo '</td>';
                        echo '<td class="rightpart">';
                            echo date('d-m-Y', strtotime($row['topic_date']));
                        echo '</td>';
                    echo '</tr>';
                }
            }
        }
    }
}
?>  
  • Well that won't work with the MySQL extension. PDO supports [prepared statements](http://php.net/manual/en/pdo.prepared-statements.php) – nice ass May 13 '13 at 14:12
  • [The Great Escapism (Or: What You Need To Know To Work With Text Within Text)](http://kunststube.net/escapism/) – deceze May 13 '13 at 14:27

1 Answers1

1

In your case surround the mysql_real_escape_string with quotes like so:

SELECT * FROM table WHERE ID = '".mysql_real_escape_String($_POST['ID'])."'

Note the extra single quotes. This will make it more secure but nevertheless its better to use prepared statements. Besides prefering to use prepared statements over mysql_query, as of php version 5.5.0 the function mysql_query() will be deprecated.

There is another topic on stackoverflow which anwsers the question 'How to prevent SQL injection in PHP Pdo'. You might find some samples and extra information : How can I prevent SQL injection in PHP?

Community
  • 1
  • 1
frank1fr
  • 129
  • 1
  • 1
  • 11