Достатъчно ли е защитен кодът?

Hous

Registered
Интересува ме според вашия опит дали може да имам "дупки" в показания код /метод и как да ги оправя,някакви предложения като цяло.Благодаря!
кодът който показва новина:
news.php
Код:
$id = (is_numeric($_GET['id']) && $_GET['id'] > 0) ? $_GET['id'] : 0;
$ssql = "SELECT * FROM news WHERE id=$id";
$result = mysql_query($ssql);
if (mysql_num_rows($result)==0) {
   echo ('News doesnt exist'); exit;
}
else if (mysql_num_rows($result)==1) {
//nasam koda za novinata
}

кода за страницирането в търсачката s_pagination.php
Код:
if($_GET)
{
$page=$_GET['page'];

// never trust what user wrote
    $secured_search = mysql_real_escape_string($_GET['search']);
    $secured_search = htmlentities($secured_search);

}

//securing the page number
if(is_numeric($page) && $page>0){
//start with the query
}


формата за търсене searchform.php
Код:
<form method="post" action="search.php" class="search_box">
    <input type="text" name="search" id="search_box" class="search_box_text" size="48"/><input type="submit" value="" class="search_button" /><br />
</form>


if (isset($_POST['search'])) {
	
    // never trust what user wrote
    $word = mysql_real_escape_string($_POST['search']);
    $word = htmlentities($word);

кода за началната страница (страницирането ме тревожи) home.php
Код:
//paging
$page = (isset($_GET['page'])) ? (int)$_GET['page'] : 0;
 
Като ще използваш само цифри, ползвай (int), така няма как да ти върне нещо различно от цифра. Не мисля, че е нужно да слагаш htmlentities след mysql_real_escape_string.

Заявките ти предлагам от:
PHP:
$ssql = "SELECT * FROM news WHERE id=$id";
да си ги направиш на:
PHP:
$ssql = "SELECT * FROM news WHERE id='".$id."'";

Също така като ще проверяш дали има $_GET, проверявай с точно определения $_GET и сложи едно isset().

Не ползвай mysql_num_rows(), а count в заявката.
 
Извинявам се ,че спамя,но мога ли да попитам защо е по-добре така

PHP:
$ssql = "SELECT * FROM news WHERE id='".$id."'";
 
critrate каза:
Извинявам се ,че спамя,но мога ли да попитам защо е по-добре така

PHP:
$ssql = "SELECT * FROM news WHERE id='".$id."'";

Щот може би така ще е по защитен :shock:

$ssql = "SELECT * FROM news WHERE id='$id'";

Това
PHP:
$id = (is_numeric($_GET['id']) && $_GET['id'] > 0) ? $_GET['id'] : 0;

ти препоръчвам да го направиш така
PHP:
$id = isset($_GET['id']) ? getNumber($_GET['id']) : NULL;

Като функцията getNumber е следната:
PHP:
	function getNumber($val) {
		$checkNumber = preg_match("/^[0-9]{0,10}$/", $val) ? $val : NULL;
		return $checkNumber;
	}
Като тук сме разрешили само цифри от 0 до 9 и стрингът може да бъде не по-дълъг от 10 символа, но естествено ти може да си го промениш от тук {0, 10}

Още нещо. Ще ти дам пример с извеждането на новина. Примерно имаш подобен линк
www.saita.com/new.php?id=1 Така ще ти изведе новина с ид 1, ами ако примерно някой направи следното
www.saita.com/new.php или
www.saita.com/new.php?id=
И 2 двата случая ще ти изведе грешка, която ти не би желал да даде, затова правиш следното
И целият ти код ще изгежда така:
PHP:
$id = isset($_GET['id']) ? getNumber($_GET['id']) : NULL;
if(!isset($id)) {
	echo "Моля изберете новина";
} else {
	$our = mysql_query("SELECT * FROM news WHERE id = '$id'") or die(mysql_error());
	while($row = mysql_fetch_assoc($our)) {
		if($row['id']) {
			//Тук вече си извеждаш новините
		} else {
			echo "Тази новина не съществува";
		}
	}
}

Лично аз ползвам този метод в проекта си по който работя в момента и няма никакви проблеми. Метода е тестван от XSS и други различни атаки и не беше открита дупка :?:

Успех !
 
Коъдът според мен е добре, но някой с по-голям Ум да го потвърди. :)
Колкото и добре да си защите, винаги ще те ханат стига да искат. Те спират правителствени сайтове, пък твоят няма да спрат. В смисъл, колкото и да си защите, винаги ще си намерят начини да ти направят мизерия. :)
 
Weezy каза:
Коъдът според мен е добре, но някой с по-голям Ум да го потвърди. :)
Колкото и добре да си защите, винаги ще те ханат стига да искат. Те спират правителствени сайтове, пък твоят няма да спрат. В смисъл, колкото и да си защите, винаги ще си намерят начини да ти направят мизерия. :)
Да но по тая логика да го оставя без никакви защити?
Все пак нали за това пуснах темата.
Като цяло има полезна информация,благодаря на всички,щре прегледам и преправя още малко кода.
Интересното нещо което намерих според w3 schools е
Код:
function check_input($value)
{
// Stripslashes
if (get_magic_quotes_gpc())
  {
  $value = stripslashes($value);
  }
// Quote if not a number
if (!is_numeric($value))
  {
  $value = "'" . mysql_real_escape_string($value) . "'";
  }
return $value;
}
....
$user = check_input($_POST['user']);
$pwd = check_input($_POST['pwd']);
$sql = "SELECT * FROM users WHERE
user=$user AND password=$pwd";
Това явно добавя и за магическите кавички ,не само за real_escape string
което според много хора в нета не е достатъчно за защита,тъй като
Escape/Quotesafe the input
MySQL, for instance, also permits \' to escape a quote, so after input of \'; DROP TABLE users; -- is "protected" by doubling the quotes, we get:
SELECT fieldlist
FROM customers
WHERE name = '\''; DROP TABLE users; --'; -- Boom!
The expression '\'' is a complete string (containing just one single quote), and the usual SQL shenanigans follow.

Направо ... нямам думи.
 

Горе