PHP include numeric

Question

If I build my pages like this do I have to check if news_id is numeric in news.php too? Or is this safe?

index.php:

    if (ctype_digit($_GET['news_id'])) include('news.php');

news.php:

    $query = mysql_query("SELECT * FROM news WHERE news_id = $_GET[news_id]");
    $row = mysql_fetch_assoc($query);

    if (!mysql_num_rows($query)) exit('The news you're trying to read do not exist.');

Answer 1

Short answer: Yes, you should.

Someone might (and will) request news.php, bypassing index.php.

Answer 2

You really should escape your data and sanitize it before sending it into MySQL. No guarantee someone won't try to send something malicious in through the post data.

$news_id = (int)$_GET[news_id];

$query = mysql_query("SELECT * FROM news WHERE news_id = " . 
                      mysql_real_escape_string($news_id));
$row = mysql_fetch_assoc($query);

if (!mysql_num_rows($query)) exit('The news you're trying to read do not exist.');

Answer 3

1. It's not safe;
2. Don't check, convert it to integer using intval();
3. Never, ever put GPC variables in SQL without escaping or casting;

Answer 4

The other answers are absolutely correct, you should never allow any user input directly into your database, or any other sensitive area.

You should validate/sanitize all input from $_GET, $_POST etc... You can use PHP’s built in filter functions or use those built into a framework such as Cake PHP or Symphony, which both make handling user data a lot easier.

jonstjohn has a good point you are leaving yourself open sql injection this way, and other forms of attack based around feeding malicious code into you application.

Worth reading Jeff Atwood’s 25 most dangerous programming mistakes for a bit of background on these issues, and others besides.