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

    +142

    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
    static OSStatus
    SSLVerifySignedServerKeyExchange(SSLContext *ctx, bool isRsa, SSLBuffer signedParams,
                                     uint8_t *signature, UInt16 signatureLen)
    {
        OSStatus        err;
        ...
    
        if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
            goto fail;
        if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
            goto fail;
            goto fail;
        if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
            goto fail;
        ...

    Говно с яблочным привкусом.
    http://habrahabr.ru/post/213525/

    P.S.: Не уверен Си это или плюсы.

    Запостил: Vindicar, 24 Февраля 2014

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

    • лол. это почти как паскалево if condition then; begin end;
      Ответить
    • показать все, что скрытоахах, я пацталомъ )
      +1
      Ответить
    • if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
              goto fail;
              goto fail;


      Чтоб наверняка? А точнее, второе гото будет безусловным.
      Ответить
    • Интересно, а что в sha-1 может вообще сфейлиться?
      Ответить
      • Это я к тому, что весь этот код - сплошное горе от ума.

        Ну не могут some_hash_update() и some_hash_final() завершиться неудачей. Тупо не могут. Там просто нечему фейлиться (кроме случая, когда через этот хеш прогнали 2^61 байт, который они по-любому забыли проверить). Поэтому никакие коды возврата и исключения тут тупо не нужны.
        Ответить
        • Это сишечка. Они могут героически проверять на NULL.
          Ответить
          • Да нету в этой проверке особого смысла...

            buf == NULL, size == 0 - все ок, просто ничего не делаем
            buf == NULL, size != 0 - аборт() или ассерт(), ибо прога явно косячная, и дальше лучше не продолжать, чтобы не запороть данные

            С тем же успехом туда могли передать не нулл а просто мусор.
            Ответить
          • Сишечка с функциями-членами?
            Ответить
        • Ну ок, есть еще случай - попутали порядок update() и final(). Но они его один хер не проверили, и эти функции всегда возвращают 0 ;)
          Ответить
        • Именно поэтому дело пахнет бэкдором. Хороший бэкдор неотличим от опечатки, а этот косяк позволял подсовывать левые сертификаты. *поправил шапочку из фольги*
          Ответить
          • А не кажется ли вам, что "goto fail;" намекает на строго противоположный сценарий?
            Ответить
            • В исходном коде код после fail преобразовывал коды ошибок в некоторых случаях. Если первые две операции прошли безошибочно, то вся функция с радостью возвращала текущий код ошибки, т.е. 0: подпись проверена успешно.
              Ответить
            • В исходном коде метка fail находилась в конце функции. Код далее тупо освобождал ресурсы и возвращал err. Таким образом, второй goto fail; означал что функция завершилась бы успешно (так как первые две стадии успешны практически всегда).

              Mea culpa, надо было это дописать.
              Ответить
              • За одинаковый отступ у двух goto надо руки отбить.
                Ответить
                • это просто паста...
                  Ответить
                  • Это не паста, это 100 пудов бекдор ("просто паста" бывает в ваших гейдевских высерах, а не в коде криптографии). И комент выше это не отменяет.

                    Да и нормальная IDE выдала бы варнинг https://twitter.com/appcode/status/437896886649757696/photo/1
                    Ответить
                    • > Да и нормальная IDE выдала бы варнинг

                      Типичная XCode не выдаёт, проверено (даже при запуске Analyze). GCC 4.8.1 не выдаёт. cppcheck 1.60.1 не выдаёт. clang и clang-check 3.4 не выдают.
                      Ответить
                      • АААА, они добрались до IDE и компилеров!
                        Ответить
                      • > нормальная IDE [...] бы
                        > XCode не [...]

                        где противоречие?
                        Ответить
          • Прочитал новость. Практически классический бекдор.
            Ответить
        • Могут завершится неудачей. Например, если используется аппаратный криптографический модуль. Или ещё какая экзотика.
          Ответить
          • в случае если железо лажается, то должен быть какой-то фолбэк на софт.

            по тому что я читал, то железо оно обламится не может - там тупое I/O. обломится может драйвер на выделении памяти или проверке входных параметров.
            Ответить
            • > драйвер на выделении памяти
              Кстати, а в линухе буфера для read/write/ioctl как драйверу передаются? Драйвер получает прямой доступ к реальному буферу в процессе пользователя, или он копируется в ядерную память на write и из ядерной памяти на read?
              Ответить
              • "Драйвер получает прямой доступ к реальному буферу в процессе пользователя, или он копируется в ядерную память на write и из ядерной памяти на read?"

                И первое и второе, но как правило второе. Потому что память процесса может быть свопнута: наивный доступ к памяти процесса чреват последствиями.
                Ответить
                • > память процесса может быть свопнута
                  Ну понятно, что есть случаи, когда к свопуемой памяти обращаться нельзя ни при каких условиях (тот же драйвер hdd, т.к. на нем может оказаться сам своп, или обработчики прерываний, где надо быстро вернуть управление). Но в общем случае, емнип, ядерная память вполне может своповаться...
                  Ответить
                  • "Но в общем случае, емнип, ядерная память вполне может своповаться..."

                    В общем случае - как раз таки нет.

                    В кернеле, есть два типа памяти: kmalloc() vs. vmalloc(). Первое быстрее и есть настоящий кусок непрерывное физической памяти (== диапазон страниц), почему и ограниченый ресурс. Второе медленее, но можно выделять почти без ограничений - но нет гарантии непрерывности физ памяти, для DMA/в обработчике прерываний/этц пользоватся такой памятью нельзя.

                    Ни первое, ни второе - не свопится. Потому что линухов кернел несвопится.

                    Для того что бы что-то сваппилось, нужно из под кернела запускать свой спец процесс (то что в top показывается в квадратных скобках) и в этом процессе теперь можно выделять свопаемую память.

                    Это краткое описание. Гугли kmalloc() и vmalloc() - на гугле лежит много всего.
                    Ответить
                    • > Потому что линухов кернел несвопится.
                      О как, я про это не знал...
                      Ответить
                      • Я не уверен что есть кернела которые можно свопать, честно говоря. Я просто не представляю себе как это будет работать.

                        Я думаю что эта байка пошла с времен OS/2 которая позволяла кернел в своп записать (потому что он сидел в обычном сегменте памяти) что многие интерпретировали как "кернел свопнут". Что само собой не соответствует реальности. Если слить на диск драйвер диска и удалить его из памяти, что же его назад из свопа прочитает?

                        Та же фигня на микро-кернелах. Сами микро-кернела нельзя свопить. И на куче внутренних процессов стоит птичка что их из памяти удалять нельзя - например тот же своппер есть обычный процесс.
                        Ответить
                        • У винды вроде бы свопуются ядерные странички, если их не пометили как несвопуемые. Но я могу и ошибаться.
                          Ответить
                          • Виндовым кернелом не интересовался. Но он вроде бы как (частично) микрокернел, поэтому "свопуются, если их не пометили как несвопуемые" вполне логичное предположение.

                            Надо будет повтыкать как-нибудь:
                            http://en.wikipedia.org/wiki/Comparison_of_operating_system_kernels
                            Ответить
            • >в случае если железо лажается, то должен быть какой-то фолбэк на софт.
              Не всегда возможно. Для вычисление хэша или какго-нибудь шифрования со сцеплением блоков необходимо получить текущее состояние железки
              Ответить
    • P.S. Реализация SSL без юнит-тестов и code review это просто ахуенно. I love you, Apple!
      Ответить
    • И что за дилетантский !=0? Для подавления ворнинга достаточно скобочек.
      Ответить
    • >Кроме того, слово fail в коде переводить не стоило, это метка.
      :))))))))))))))))))))))))))))))))))))))) )
      Ответить
    • https://twitter.com/appcode/status/437896886649757696/photo/1
      Ответить
    • Я щитаю, надо было делать язык/насаждать кодстайл с максимальным TOOWTDI.
      А именно ОБЯЗАТЕЛЬНО ставить операторные скобки. Конечно не панацея, но в разы снижает вероятность всяких проёбов.
      Ответить
      • А лучше сразу отступы, как в питоне, бггг.
        Ответить
      • Проще сделать старндарт - язык плюс автоформатер кода. какое бы говно не написал сначала оно форматируется, а только потом компилится.
        Ответить
        • > какое бы говно не написал сначала оно форматируется, а только потом компилится
          Да принудительное форматирование и сейчас вроде как есть.

          Что совсем не отменяет тестирования. Особенно в таких ответственных местах.
          Ответить
          • ну как принудительное - если не хочешь - оно не отформатирует. Я часто вижу в шарпокоде что поля объявляются там, где они понадобились (т.е. метод-поле-метод-метод-поле и т.д.). И как им объяснить как надо жить?
            Ответить
    • показать все, что скрытоvanished
      Ответить
      • Я вообще не использую отступы - для меня они являются не чем иным, как мусором, затрудняющим восприятие кода; кроме того, из-за них деградирует структурное мышление, ибо мозгу незачем напрягаться, когда влажные пробелы и табуляции расставлены.
        Ответить
        • показать все, что скрытоvanished
          Ответить
          • В нём нет смысла. Когда мне всё же пришлось, поминутно заглядывая в документацию, править в чьём-то проекте скрипт на "Python", являющийся "WebSocket"-сервером, я был в глубоком ахуе - зачем, когда есть чудненький "Workerman" на "PHP"? "PHP" уже далеко не такой импотентный, каким мог показаться ещё лет пять-шесть назад. Незачем цепляться за убогие язычки.
            Ответить
        • Твои сообщения являются не чем иным, как мусором, затрудняющим восприятие сайта говнокод.ру

          > из-за них деградирует структурное мышление, ибо мозгу незачем напрягаться

          А из-за подсветки синтаксиса вообще наверное весь мозг отмирает за ненадобностью.

          То-то я смотрю твой разум как накачанный мускул блять, от таких суровых испытаний, каждый день пишешь неподдерживаемое говно в блокноте без отступов, и посмеиваешься, пока всякие борманды деградируют со своими удобными инструментиками.
          Ответить
          • Как писалось в детской книге "Повесть о Ходже Насреддине": "Да будет проклята постель, на которой я зачал тебя".
            Ответить

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