amwd07 Posted October 22, 2008 Share Posted October 22, 2008 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 .... } Quote Link to comment Share on other sites More sharing options...
DarkWater Posted October 22, 2008 Share Posted October 22, 2008 It should be: if (!isset($id) or !isValidID($id)) { Quote Link to comment Share on other sites More sharing options...
philipolson Posted October 22, 2008 Share Posted October 22, 2008 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. Quote Link to comment Share on other sites More sharing options...
DarkWater Posted October 22, 2008 Share Posted October 22, 2008 Yeah, probably. Quote Link to comment Share on other sites More sharing options...
amwd07 Posted October 22, 2008 Author Share Posted October 22, 2008 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'>"; Quote Link to comment Share on other sites More sharing options...
philipolson Posted October 22, 2008 Share Posted October 22, 2008 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 Quote Link to comment Share on other sites More sharing options...
amwd07 Posted October 23, 2008 Author Share Posted October 23, 2008 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'>"; ?> Quote Link to comment Share on other sites More sharing options...
amwd07 Posted October 23, 2008 Author Share Posted October 23, 2008 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'>"; } ?> Quote Link to comment Share on other sites More sharing options...
philipolson Posted October 23, 2008 Share Posted October 23, 2008 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. Quote Link to comment Share on other sites More sharing options...
discomatt Posted October 23, 2008 Share Posted October 23, 2008 - 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() Quote Link to comment Share on other sites More sharing options...
philipolson Posted October 23, 2008 Share Posted October 23, 2008 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. Quote Link to comment Share on other sites More sharing options...
discomatt Posted October 23, 2008 Share Posted October 23, 2008 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. Quote Link to comment Share on other sites More sharing options...
DarkWater Posted October 23, 2008 Share Posted October 23, 2008 <?php $q = sprintf("SELECT * FROM table WHERE id = %d", (int) $_GET['id']); ?> That can't be exploited either. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.