1. PHP / Говнокод #7608

    +169

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    25. 25
    26. 26
    27. 27
    28. 28
    29. 29
    30. 30
    case"register": // если do=register, выводим регистрацию
         if (isset($login) && isset($pass1) && isset($pass2)) {
           if (!empty($login) && !empty($pass1) && !empty($pass2)) {
               $users=get_serial('users');
               $reallogin=$login;
               $login=md5(strtolower($login));
               if (!$users[$login]) {
                   if (strlen($pass1)>=4) {
                       $pass1=md5($pass1);
                       $pass2=md5($pass2);
                       if ($pass1==$pass2) {
                           $users[$login]=array();
                           $users[$login]['login']=htmlspecialchars($reallogin);
                           $users[$login]['pass']=$pass1;
                             set_serial($users,'users');
                             $error="Вы успешно зарегистрированны";
                             header("Refresh:3;url=".$_SERVER['PHP_SELF']);
                       }else {
                           $error="Ошибка: Пароли не совпадают";
                       }
                   }else {
                       $error="Ошибка: Минимальная длина пароля 4 символа";
                   }
               } else {
                   $error="Ошибка: Такой пользователь уже существует";
               }
           }else {
             $error="Ошибка: Обязательные поля нужно заполнить";
           }
         }

    Запостил: invision70, 21 Августа 2011

    Комментарии (19) RSS

    • мило. все перлы начинающего в одном:
      1. один большой index.php на все случаи жизни, с главным "циклом жизни" switch-case
      2. условноисключающая валидация if-else
      3. сначала проверка на isset - и тут же на !empty
      4. непоследовательный API в get_serial('users') и set_serial($users,'users')
      5. куча ненужных md5 без засолки
      6. куча лишних "выделений", как в 5ой или 12ой строке
      7. двойные кавычки *WALL* для простых строк
      8. НАСТОЯЩИЙ логин! хотя, может, и имеет смысл кодировать в базу и логин?
      9. 17ая строка заслуживает особого внимания
      9.1. редирект не через Location, а через Refresh
      9.2. редирект со всеми GETными параметрами - в случае form method="GET" вероятно К.З.
      9.3. и КОНТРОЛЬНЫЙ: PHP_SELF уязвимость....

      я ничего не забыл? )))
      Ответить
      • Я вот вижу регистер_глобалс, мой юный падаван.

        >сначала проверка на isset - и тут же на !empty
        Ого, а у нас теперь существование переменной - уже знак того, что она не пуста?

        >КОНТРОЛЬНЫЙ: PHP_SELF уязвимость....
        Что? Гугл про уязвимость в $_SERVER['PHP_SELF'] знает только одну страницу - эту.
        Ответить
        • это необязательно регистер_глобалс, там еще вначале может быть
          if(isset($_REQUEST['login'])) $login=$_REQUEST['login'];
          и куча подобного (может, $_GET или $_POST)

          конструкция empty также не кидает нотисов, если переменная не определена, поэтому можно приравнять "не существует" и "пуста"
          Ответить
          • Но мне хотелось бы услышать про уязвимость в $_SERVER['PHP_SELF'].
            Ответить
            • если содержится \n то можно будет послать еще пару любых заголовков, в т.ч. перенаправление на любой сайт в контексте этого (с куками, идентификатором сессии и т.д.)
              Ответить
              • И как же я могу изменить значение $_SERVER['PHP_SELF']?
                Ответить
      • strlen($pass1) не правильно посчитает кириллицу в UTF-8 пароль аб пройдет, минимальная длина пароля за хардкожена. И это просто $reallogin=$login; эх .....
        Ответить
      • Подскажите, а чем лучше заменить этот "цикл жизни" switch-case? у меня вот тоже самое во всех индексах сайтов.
        Ответить
    • ах да, и еще "зарегистрированны"
      Ответить
    • вспомнилось
      http://govnokod.ru/2545
      Ответить
      • А мне вспомнилось if ($error == "Все хорошо")
        Ответить
    • Сравнение хешей двух паролей, конечно, сильно.

      Но главная беда тут в юзабилити. За такие интерфейсы, где одно опущенное поле показывает одну ошибку и сбрасывает форму - надо убивать.
      Проверять нужно все значения в форме, диагностические сообщения должны быть для каждого поля, форма сбрасываться не должна (возможно, за исключением поля "пароль", но это вопрос спорный).
      Ответить
      • http://ragecomics.cloudapp.net/Troll/23954/password-captcha-troll
        Ответить
      • > Сравнение хешей двух паролей
        Сидимо, где-то глубоко в мозгу автора отпечаталось, что работать с паролями напрямую - зло, нужно всегда использовать хэши. На всякий случай.
        Ответить
    • $error="Вы успешно зарегистрированны";

      с намеком:)
      Ответить
      • "Регистрироваться у нас - уже большая ошибка..."
        Ответить

    Добавить комментарий