1. Си / Говнокод #3575

    +140

    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
    int 
    grub_auth_strcmp (const char *user_input, const char *template) 
    { 
      int ok = 1; 
      const char *ptr1, *ptr2; 
      for (ptr1 = user_input, ptr2 = template; *ptr1; ptr1++) 
        if (*ptr1 == (ptr2 ? *ptr2 : ptr1[1]) && ok && ptr2 != NULL) 
          ptr2++; 
        else 
          ok = 0; 
     
      return !ok; 
    }

    Несвежий говнокод (давно пропатчено) и, возможно, кто-то скажет "баян", однако оставлю это здесь.
    Код из загрузчика grub 1.97, проверка пароля. Принимает за верный пароль любую подстроку пароля.

    Запостил: cfdev, 27 Июня 2010

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

    • То, что забыли поставить звёздочки у "ptr2" - с кем не бывает :). А вот проверка странная. Не понятно зачем *ptr1 сравнивать с ptr1[1]. В худшем случае, лучше уж сравнили бы с нулём, т.к. известно, что *ptr1 не может был равен нулю в момент попадания на if. Другими словами, лучше бы сделали скобочку у "if" хотя бы такой:
      (*ptr1 == *ptr2 && ok)

      Ещё меня лично напрягает, когда цикл проходит до конца, не смотря на то, что ok уже может быть выставлен на ноль. Я бы прерывал цикл при первом же несовпадении.
      Ответить
      • а не напрягает, что это велосипед?
        Ответить
        • Напрягает, но вдруг это промежуточный вариант прежде чем реализовать велосипед с какой-то фишкой? По названию функции видно, что авторы кода знали о "strcmp()", однако зачем-то написали.

          Хотя вы правы, конкретно в данной реализации, данная функция нах не нужна, в худшем случае можно было и заглушку поставить, если это промежуточный вариант функции. :(
          Ответить
          • > по названию функции видно, что авторы кода знали о "strcmp()", однако зачем-то написали.

            Чувак, это загрузка ядра. libc там не очень как бы доступен.
            Ответить
            • Да, я идиот :)). Забыл задуматься над тем, что делает grub)
              Ответить
            • могли бы тупо скопировать strcmp откуда-нибудь
              Ответить
      • > Я бы прерывал цикл при первом же несовпадении.

        Разрабы grub'а мучаются насчёт того, что по длительности работы этой функции можно определить, какой символ не совпал (грубо говоря, не совпал первый символ - функция проработала 1 мс, не совпал десятый - 10 мс). В новой реализации они на что-то вроде (100 - n) мс делают sleep :)
        Ответить
    • Любую начальную подстроку, может? А то больно мало для Кнута-Морриса-Пратта.
      Ответить
    • в строке (*ptr1 == (ptr2 ? *ptr2 : ptr1[1]) && ok && ptr2 != NULL) *ptr1 всегда будет сравниваться с *ptr2 из-за условия ptr2 != NULL
      где ошибка в логике? почему принимается любвя подстрока?
      Ответить
    • Идея была создать цикл, который всегда берет одинаково времени, вне зависимости от данных. Проблема была в том, что компилятор, как положенно, оптимизировал. В результате функция получилась слишком сложной и в ней оказался баг. В новой версии это было упрощенно и исправлено.
      Ответить

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