$_GET Security

[amwd07]

Hi

hopefully someone can help or advice me here please.

 

I am using the EZSQL MYSQL class & would like to query the ID based on the URL

Not sure if the below is secure?

 

$id = $_GET['content_id'];
$content = $db->get_row("SELECT `heading`,`desc`,`desc` FROM `content` WHERE `content_id` = '".intval($id)."'");

 

also I would like to try to get this function to work which I think would make this more secure

again not sure if this is the right solution.

 

open for suggestions on this one

 

function isValidID($id)
{
	$id = intval($id);
	return is_numeric($id) AND $id > 0 AND $id < 4294967296;
}

if (!isset($id) OR isValidID($id))
$errors = 'No hackers!';
else {

content here ....

}

[DarkWater]

It should be:

 

if (!isset($id) or !isValidID($id)) {

[philipolson]

And assuming you do $id = $_GET['content_id'] above it then $id will ALWAYS be set so I think you mean empty() and not isset() here.

[DarkWater]

Yeah, probably.

[amwd07]

sorry might be mis understanding this one, also realise wrong fields were queried

 

$id = $_GET['content_id'];
$content = $db->get_row("SELECT `section_heading`,`short_desc`,`main_desc` FROM `content` WHERE `content_id` = '".intval($id)."'");

function isValidID($id)
{
	$id = intval($id);
	return is_numeric($id) AND $id > 0 AND $id < 4294967296;
}

if (!empty($id) or !isValidID($id)) {
$error = 'No hackers!';
echo $error;
else {

echo "<div id='main'>";

[philipolson]

A few notes:

 

- You execute the query before the check, so really the use of isValidID() is useless here.

- Change !empty to empty.

- Since you use intval() it will pass the is_numeric() check 100% of the time

- For INT, don't use '' in the query around it.

- Feel free to place your function definition somewhere else, it can be defined after use

[amwd07]

All I need to do here is only allow validate ID in the DB to make this more secure

 

<?php // Get Main Content from DB
$id = $_GET['content_id'];

$content = $db->get_row("SELECT `section_heading`,`short_desc`,`main_desc` FROM `content` WHERE `content_id` = '".intval($id)."'");
$content_id = $content->content_id;

    /* function isValidID($id)
{
	$id = intval($id);
	return is_numeric($id) AND $id > 0 AND $id < 1000;
}
    */

if (empty($content_id)) {
$error = 'No hackers!';
echo $error;
} else {

echo "<div id='main'>";

?>

[amwd07]

any one any idea's how to get this issue resolved.

I have now changed the code but still not working ???

 

<?php // Get Main Content from DB

	function ValidGet($var,$type,$default=false) {
	$var_value == $_GET[$var];
		switch($type){
		 case 'INT':
		   $var_value = preg_replace("/[^0-9]/","",$var_value);  
		  break;
		}		
	}

if($id = ValidGet('content_id','INT')) {
    $content = $db->get_row("SELECT `section_heading`,`short_desc`,`main_desc` FROM `content` WHERE `content_id` = '".$id."'");
}

$db->debug();

if (empty($id) || !$content) {
$error = 'No hackers!';
echo $error;
} else {
echo "<div id='main'>";

}
?>

[philipolson]

To be blatantly honest, your code is wrong on so many levels that it's difficult to know where to begin but perhaps some kind soul with more patience will respond. I suggest you lose most of this code and simply use intval(), so, replace .$id. with .intval($id). and in the meantime read up on how to define and use functions.

[discomatt]

- For INT, don't use '' in the query around it.

 

For the record, using quotes around an INT value is perfectly acceptable in MySQL... it's actually recommended from a security standpoint, as it can help prevent injection that gets around mysql_real_escape_string()

[philipolson]

But an INT is an INT, so I disagree that it's recommended or better here to use 'quotes' for it. intval() is a straight forward function. Life goes on, not a big deal, query works both ways, but I wanted to clarify.

[discomatt]

Agreed, with proper sanitizing, quotes are not needed for integers.

 

I'm simply saying

 

<?php

$id = mysql_real_escape_string( $_GET['id'] );

$q = "SELECT * FROM `table` WHERE `id` = $id";

?>

 

Can be exploited, while

 

<?php

$id = mysql_real_escape_string( $_GET['id'] );

$q = "SELECT * FROM `table` WHERE `id` = '$id'";

?>

 

Can't.

[DarkWater]

<?php
$q = sprintf("SELECT * FROM table WHERE id = %d", (int) $_GET['id']);
?>

 

That can't be exploited either.