Вот че-то пишу.

Собсно вот. В продолжении темы о шаблонизаторах и прочем.

Прошу рассказать о возможных уязвимостях и о том, как оптимизировать код. Ну и о переводе его на шаблонизатор.
index.php

PHP:

1. <?php
2. define("MAIN_FILE", true);
3. include'core/mysql.php';
4. include'core/auth.php';
5.  
6. showLogInForm('admin');
7. echo isAdmin();
8. ?>
9. <style type="text/css">
10. form { margin: 0; }
11. </style>

auth.php

PHP:

1. <?php
2. if (!defined("MAIN_FILE")) die("Illegal File Access");
3. define("COOKIE_SALT", 'abcdef');
4.  
5. define("PRIV_SALE_REGISTER", 1);
6. define("PRIV_SALE_SHOW", 2);
7. define("PRIV_SALE_EDIT", 3);
8. define("PRIV_SALE_CONFIRM", 4);
9. /**/
10. function isUser(){
11. global $db;
12.     $result = $db->sql_query("SELECT name, password, salt, privileges FROM mypr_user WHERE id='".$_COOKIE['id']."'");
13.     list($name, $passwordDB, $salt, $privileges) = $db->sql_fetchrow($result);
14.     if (md5($passwordDB . COOKIE_SALT)==$_COOKIE['user']) {
15.         return $privileges;
16.     }
17.     return false;
18. }
19. /**/
20. function isAdmin(){
21. global $db;
22.     $result = $db->sql_query("SELECT * FROM mypr_admin WHERE id='".$_COOKIE['id']."'");
23.     list($id, $name, $passwordDB, $salt, $privileges) = $db->sql_fetchrow($result);
24.     if (md5($passwordDB . COOKIE_SALT)==$_COOKIE['admin']) {
25.         return $privileges;
26.     }
27.     return false;
28. }
29. /**/
30. function logIn($logInType, $name, $password){
31. global $db;
32.     switch($logInType){
33.         case "admin":
34.             $result = $db->sql_query("SELECT * FROM mypr_admin WHERE name='".$name."'");
35.             list($id, $name, $passwordDB, $salt, $privileges) = $db->sql_fetchrow($result);
36.             if (md5(md5($password) . $salt)==$passwordDB) {
37.             setcookie('admin', md5($passwordDB . COOKIE_SALT), time()+86400);
38.             setcookie('id', $id, time()+86400);
39.         }
40.         break;
41.     case "user":
42.         $result = $db->sql_query("SELECT id, name, password, salt FROM mypr_user WHERE name='".$name."'");
43.             list($id, $name, $passwordDB, $salt) = $db->sql_fetchrow($result);
44.             if (md5(md5($password) . $salt)==$passwordDB) {
45.             setcookie('user', md5($passwordDB . COOKIE_SALT), time()+86400);
46.             setcookie('id', $id, time()+86400);
47.         }
48.         break;
49.     }
50.     header("Location: index.php");
51. }
52. /**/
53. function logOut($logInType){
54.     switch($logInType){
55.         case "admin":
56.         setcookie('admin', '', time()-86400);
57.         break;
58.     case "user":
59.         setcookie('user', '', time()-86400);
60.         break;
61.     }
62. }
63. /**/
64. function showLogInForm($logInType='user'){
65.     ?>
66.     <div id="divLogInForm" style="text-align:center">
67.          Представьтесь, пожалуйста.
68.          <form name="logInForm" id="logInFormID" action="?do=logIn" method="post">
69.          Имя пользователя: <input type="text" name="name"><br />
70.          Пароль: <input type="password" name="password"><br />
71.          <?
72.          if ($logInType=='admin') {
73.              ?>
74.              <input type="hidden" name="admin" value="yes">
75.              <?
76.          }
77.          ?>
78.          <input type="submit" value="Вход">
79.      </form>
80.     </div>
81.     <?
82. }
83.  
84. switch($_GET['do']) {
85.     case "logIn":
86.         $logInType = ($_POST['admin']=='yes') ? 'admin' : 'user';
87.         logIn($logInType, $_POST['name'], $_POST['password']);
88.     break;
89. }
90.  
91. ?>

 

Elkaz
да, вот и мне внутренний голос говорит, что нужно как-то фильтровать "плохие символы" перед тем как с базой сравнивать. Но как именно я не знаю.

А что с глобальными переменными делать?
заменить

PHP:

1. <?logIn($logInType, $_POST['name'], $_POST['password']);?>

на

PHP:

1. <?logIn($logInType, $_POST['name'], $_POST['password'], $db);?>

?

В догонку функция запроса в БД:

PHP:

1. <?
2.     function sql_query($query = "", $transaction = false) {
3.         unset($this->query_result);
4.         if ($query != "") {
5.             $st = array_sum(explode(" ", microtime()));
6.             $this->query_result = @mysql_query($query, $this->db_connect_id);
7.             $total_tdb = round(array_sum(explode(" ", microtime())) - $st, 5);
8.             $this->total_time_db += $total_tdb;
9.             $this->time_query .= "".$total_tdb > 0.01."" ? "<font color=\"red\"><b>".$total_tdb."</b></font> "._SEC.". - [".$query."]<br /><br />" : "<font color=\"green\"><b>".$total_tdb."</b></font> "._SEC.". - [".$query."]<br /><br />";
10.         }
11.         if ($this->query_result) {
12.             $this->num_queries += 1;
13.             unset($this->row[$this->query_result]);
14.             unset($this->rowset[$this->query_result]);
15.             return $this->query_result;
16.         } else {
17.             return ($transaction == END_TRANSACTION) ? true : false;
18.         }
19.     }
20.  
21. ?>

 

Горбунов Олег во как). Смеялся в голос про подпись. А magic_quotes_gpc не хватит?

 

http://ua.php.net/mysql_real_escape_string

PHP:

1. <?php
2. // Функция экранирования переменных
3. function quote_smart($value)
4. {
5.     // если magic_quotes_gpc включена - используем stripslashes
6.     if (get_magic_quotes_gpc()) {
7.         $value = stripslashes($value);
8.     }
9.     // Если переменная - число, то экранировать её не нужно
10.     // если нет - то окружем её кавычками, и экранируем
11.     if (!is_numeric($value)) {
12.         $value = "'" . mysql_real_escape_string($value) . "'";
13.     }
14.     return $value;
15. }
16.  
17. // Соединяемся
18. $link = mysql_connect('mysql_host', 'mysql_user', 'mysql_password')
19.     OR die(mysql_error());
20.  
21. // Составляем безопасный запрос
22. $query = sprintf("SELECT * FROM users WHERE user=%s AND password=%s",
23.             quote_smart($_POST['username']),
24.             quote_smart($_POST['password']));
25.  
26. mysql_query($query);
27. ?>

понравилось.

upd: Хотя имхо это параноя так защищаться от инъекции. Как бы упростить решение, не пренебрегая безопастностью??

upd2: Хотя, чтоб не стать быдлокодером, наверно нужно так и делать.. [чешет затылок]

 

вот сваял кой-че:

PHP:

1. <?php
2. function clean($variable) {
3.     if(is_numeric($variable))
4.         return $variable;
5.     if(!get_magic_quotes_gpc()) {
6.         if(is_array($variable)){
7.             foreach($variable as $k=>$v){
8.                 $variable[$k]=clean($v);
9.             }  
10.          return $variable;
11.         }
12.         return "'" . mysql_real_escape_string($variable) . "'";
13.     }
14.     return "'" . $variable . "'";
15. }
16. ?>

Код (Text):

1. запускаю http://localhost/?aa=aa'aa&bb[11][22]=aa'3`3

проверяю

PHP:

1. <?php
2. echo $_GET['aa'],'<br />';
3. echo clean($_GET['aa']),'<br />';
4.  
5. echo $_GET['bb'][11][22],'<br />';
6. $tmp = clean($_GET['bb']);
7. echo $tmp[11][22],'<br />';
8. ?>

php_flag magic_quotes_gpc off
php_flag magic_quotes_runtime off
aa'aa
'aa\'aa'
aa'3`3
'aa\'3`3'

php_flag magic_quotes_gpc on
php_flag magic_quotes_runtime on
aa\'aa
'aa\'aa'
aa\'3`3
'aa\'3`3'

It working!

Задался вопросом: mysql_real_escape_string или addslashes ? что лучше, что быстрее? Думаю, что второе, так как не требуется загружать расширение для работы с БД.

 

PHP:

1. <?
2. //это
3. $result = $db->sql_query("SELECT * FROM mypr_admin WHERE id='".$_COOKIE['id']."'");
4.  
5. $result = $db->sql_query("SELECT id, name, password, salt FROM mypr_user WHERE name='".$name."'");
6.  
7.  
8. //заменил на это
9. $sql = sprintf("SELECT * FROM mypr_admin WHERE id=%d", $_COOKIE['id']);
10. $result = $db->sql_query($sql);
11.  
12. $sql = sprintf("SELECT id, name, password, salt FROM mypr_user WHERE name=%s",clean($name));
13. $result = $db->sql_query($sql);
14.  
15. ?>

ну как? так правильно?

 

нет. Его sprintf преобразует в int. Или не преобразует?

Немного изменил код:

PHP:

1. <?php
2. // $rev 20.05.08
3. // экранирует спецсимволы внутри переменной вне зависимости от
4. //  настроек magic_quotesи обрамляет переменную одинарными кавычками,
5. //  если она не является числом
6. function cleanQ($variable) {
7.     // $rev 20.05.08
8.     //обрамляет переменную одинарными кавычками. Видна только из cleanQ
9.     if (!function_exists('addQ')) {
10.         function addQ($variable){
11.             if(is_array($variable)){
12.             foreach($variable as $k=>$v){
13.                 $variable[$k]=addQ($v);
14.             }
15.             return $variable;
16.         }
17.         return "'" . $variable . "'";
18.         }
19.     }
20.    
21.     if(is_numeric($variable))
22.         return $variable;
23.     if(!get_magic_quotes_gpc()) {
24.         if(is_array($variable)){
25.             foreach($variable as $k=>$v){
26.                 $variable[$k]=cleanQ($v);
27.             }  
28.          return $variable;
29.         }
30.         //return "'" . mysql_real_escape_string($variable) . "'";
31.         return "'" . addslashes($variable) . "'";
32.     }
33.     return addQ($variable);
34. }
35. // $rev 20.05.08
36. // экранирует спецсимволы внутри переменной вне зависимости от настроек magic_quotes
37. function clean($variable) {
38.     if(is_numeric($variable))
39.         return $variable;
40.     if(!get_magic_quotes_gpc()) {
41.         if(is_array($variable)){
42.             foreach($variable as $k=>$v){
43.                 $variable[$k]=clean($v);
44.             }  
45.          return $variable;
46.         }
47.         //return mysql_real_escape_string($variable);
48.      return addslashes($variable);
49.     }
50.     return $variable;
51. }
52. ?>

 

апдейт. Как-то мне нужно было обрамить кавычками число. Уже не помню, за каким чертом мне это понадобилось, но было нужно. cleanQ(1, 1);

PHP:

1. <?php
2. // $rev 29.06.08
3. // экранирует спецсимволы внутри переменной вне зависимости от
4. //  настроек magic_quotes и обрамляет переменную одинарными кавычками,
5. //  даже если она является числом ($bringQ = 1)
6. function cleanQ($var, $bringQ = 0) {
7.     // $rev 29.06.08
8.     //обрамляет переменную одинарными кавычками. Видна только из cleanQ
9.     if (!function_exists('addQ')) {
10.         function addQ($var){
11.             if(is_array($var)){
12.                 foreach($var as $k=>$v){
13.                     $var[$k]=addQ($v);
14.                 }
15.                 return $var;
16.             }
17.             return "'" . $var . "'";
18.         }
19.     }
20.    
21.     if(is_numeric($var) && !$bringQ)
22.         return $var;
23.     if(!get_magic_quotes_gpc()) {
24.         if(is_array($var)){
25.             foreach($var as $k=>$v){
26.                 $var[$k]=cleanQ($v);
27.             }  
28.          return $var;
29.         }
30.         //return "'" . mysql_real_escape_string($var) . "'";
31.         return "'" . addslashes($var) . "'";
32.     }
33.     return addQ($var);
34. }
35. // $rev 20.05.08
36. // экранирует спецсимволы внутри переменной вне зависимости от настроек magic_quotes
37. function clean($var) {
38.     if(is_numeric($var))
39.         return $var;
40.     if(!get_magic_quotes_gpc()) {
41.         if(is_array($var)){
42.             foreach($var as $k=>$v){
43.                 $var[$k]=clean($v);
44.             }  
45.          return $var;
46.         }
47.         //return mysql_real_escape_string($var);
48.      return addslashes($var);
49.     }
50.     return $var;
51. }
52. ?>