Стреляем в ногу, обрабатывая входные данные


    Связующее звено сегодняшней статьи отличается от обычного. Это не один проект, для которого был проведён анализ исходного кода, а ряд срабатываний одного и того же диагностического правила в нескольких разных проектах. В чём здесь интерес? В том, что некоторые из рассмотренных фрагментов кода содержат ошибки, воспроизводимые при работе с приложением, а другие – и вовсе уязвимости (CVE). Кроме того, в конце статьи немного порассуждаем на тему дефектов безопасности.

    Краткое предисловие


    Все ошибки, которые будут рассмотрены сегодня в статье, имеют схожий паттерн:

    • программа принимает данные из потока stdin;
    • выполняется проверка успешности считывания данных;
    • если данные считаны успешно, из строки удаляется символ переноса.

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

    Все проблемы, приведённые ниже, были обнаружены статическим анализатором PVS-Studio, который ищет ошибки в коде не только для C, C++, но и для C#, Java.

    Конечно, найти проблему статическим анализатором – хорошо, но найти и воспроизвести – это уже совершенно другой уровень удовольствия. :)

    FreeSWITCH


    Первый подозрительный фрагмент кода был обнаружен в коде модуля fs_cli.exe, входящего в состав дистрибутива FreeSWITCH:

    static const char *basic_gets(int *cnt)
    {
      ....
      int c = getchar();
      if (c < 0) {
        if (fgets(command_buf, sizeof(command_buf) - 1, stdin) 
              != command_buf) {
          break;
        }
        command_buf[strlen(command_buf)-1] = '\0'; /* remove endline */
        break;
      }
      ....
    }

    Предупреждение PVS-Studio: V1010 CWE-20 Unchecked tainted data is used in index: 'strlen(command_buf)'.

    Анализатор предупреждает о подозрительном обращении по индексу к массиву command_buf. Подозрительным оно считается по той причине, что в качестве индекса используются непроверенные внешние данные. Внешние – потому что получены через функцию fgets из потока stdin. Непроверенными — так как никакой проверки перед использованием выполнено не было. Выражение fgets(command_buf, ....) != command_buf — не в счёт, так как таким образом мы проверяем только факт получения данных, но не их содержимое.

    Проблема данного кода в том, что при определённых условиях произойдёт запись '\0' за пределы массива, что приведёт к возникновению неопределённого поведения. Для этого достаточно ввести строку нулевой длины (строку нулевой длины с точки зрения языка Си, то есть такую, в которой первым символом будет '\0').

    Давайте прикинем, что произойдёт, если передать на вход строку нулевой длины:

    • fgets(command_buf, ....) -> command_buf;
    • fgets(....) != command_buf -> false (then-ветвь оператора if игнорируется);
    • strlen(command_buf) -> 0;
    • command_buf[strlen(command_buf) — 1] -> command_buf[-1].

    Упс!

    Интересно здесь то, что это предупреждение анализатора вполне себе можно «пощупать руками». Для того, чтобы повторить проблему, нужно:

    • довести исполнение программы до данной функции;
    • подстроить ввод таким образом, чтобы вызов getchar() вернул отрицательное значение;
    • передать для функции fgets строку с терминальным нулём в начале, которую та должна успешно считать.

    Немного покопавшись в исходниках, я составил конкретную последовательность воспроизведения проблемы:

    • Запускаем fs_cli.exe в batch-режиме (fs_cli.exe -b). Отмечу, что для выполнения дальнейших шагов подключение fs_cli.exe к серверу должно пройти успешно. Для этого достаточно, например, локально запустить FreeSwitchConsole.exe от имени администратора.
    • Осуществляем ввод, чтобы вызов getchar() вернул отрицательное значение.
    • Вводим строку с терминальным нулём в начале (например, '\0Oooops').
    • ....
    • PROFIT!

    Ниже приводится видеозапись воспроизведения проблемы:


    NcFTP


    В проекте NcFTP была обнаружена аналогичная проблема, только встретилась она уже в двух местах. Так как код выглядит похоже, рассмотрим только одно проблемное место:

    static int NcFTPConfirmResumeDownloadProc(....)
    {
      ....
      if (fgets(newname, sizeof(newname) - 1, stdin) == NULL)
        newname[0] = '\0';
      newname[strlen(newname) - 1] = '\0';
      ....
    }

    Предупреждение PVS-Studio: V1010 CWE-20 Unchecked tainted data is used in index: 'strlen(newname)'.

    Здесь, в отличии от примера из FreeSWITCH, код написан хуже и больше подвержен проблемам. Например, запись '\0' происходит вне зависимости от того, успешно произошло считывание с использованием fgets или нет. То есть здесь даже больше возможностей того, как можно нарушить нормальную логику исполнения. Пойдём проверенным путём – через строки нулевой длины.

    Воспроизводится проблема немного сложнее, чем в случае с FreeSWITCH. Последовательность шагов описана ниже:

    • запуск и подключение к серверу, с которого можно загрузить файл. Я, например, использовал speedtest.tele2.net (в итоге команда запуска приложения выглядит так: ncftp.exe speedtest.tele2.net);
    • загрузка файла с сервера. Локально уже должен существовать файл с таким же именем, но с другими свойствами. Можно, например, загрузить файл с сервера, изменить его, и попробовать снова выполнить команду загрузки (например, get 512KB.zip);
    • на вопрос о выборе действия ответить строкой, начинающейся с символа 'N' (например, Now let's have some fun);
    • ввести '\0' (или что-нибудь поинтереснее);
    • ....
    • PROFIT!

    Воспроизведение проблемы также записано на видео:


    OpenLDAP


    В проекте OpenLDAP (точнее – в одной из сопутствующих утилит) наступили на те же грабли, что и во FreeSWITCH. Попытка удаления символа переноса строки происходит только при условии, что строка была считана успешно, но защиты от строк нулевой длины также нет.

    Фрагмент кода:

    int main( int argc, char **argv )
    {
      char buf[ 4096 ];
      FILE *fp = NULL;
      ....
      if (....) {
        fp = stdin;
      }
      ....
      if ( fp == NULL ) {
        ....
      } else {
        while ((rc == 0 || contoper)
               && 
               fgets(buf, sizeof(buf), fp) != NULL) {
          buf[ strlen( buf ) - 1 ] = '\0'; /* remove trailing newline */
    
          if ( *buf != '\0' ) {
            rc = dodelete( ld, buf );
            if ( rc != 0 )
              retval = rc;
            }
          }
      }
      ....
    }

    Предупреждение PVS-Studio: V1010 CWE-20 Unchecked tainted data is used in index: 'strlen(buf)'.

    Выкинем лишнее, чтобы суть проблемы стала более очевидна:

    while (.... && fgets(buf, sizeof(buf), fp) != NULL) {
      buf[ strlen( buf ) - 1 ] = '\0';
      ....
    }

    Этот код лучше, чем в NcFTP, но всё равно является уязвимым. Если при запросе fgets передать на вход строку нулевой длины:

    • fgets(buf, ....) -> buf;
    • fgets(....) != NULL -> true (начинает исполняться тело цикла while);
    • strlen(buf) — 1 -> 0 — 1 -> -1;
    • buf[-1] = '\0'.

    libidn


    Несмотря на то, что ошибки, разобранные выше, достаточно интересны (они стабильно воспроизводятся, и их можно 'пощупать' (разве что до воспроизведения проблемы на OpenLDAP у меня руки не дотянулись)), уязвимостями их назвать нельзя хотя бы по той причине, что проблемам не присвоены CVE-идентификаторы.

    Тем не менее, такой же паттерн проблемы имеют и некоторые настоящие уязвимости. Оба фрагмента кода, приведённые ниже, относятся к проекту libidn.

    Фрагмент кода:

    int main (int argc, char *argv[])
    {
      ....
      else if (fgets (readbuf, BUFSIZ, stdin) == NULL)
      {
        if (feof (stdin))
          break;
      
        error (EXIT_FAILURE, errno, _("input error"));
      }
    
      if (readbuf[strlen (readbuf) - 1] == '\n')
        readbuf[strlen (readbuf) - 1] = '\0';
      ....
    }

    Предупреждение PVS-Studio: V1010 CWE-20 Unchecked tainted data is used in index: 'strlen(readbuf)'.

    Ситуация похожа, за исключением того, что в отличии от предыдущих примеров, где осуществлялась запись по индексу -1, здесь происходит чтение. Тем не менее, это всё ещё undefined behavior. Данная ошибка удостоилась собственного идентификатора CVE (CVE-2015-8948).

    После обнаружения проблемы данный код изменили следующим образом:

    int main (int argc, char *argv[])
    {
      ....
      else if (getline (&line, &linelen, stdin) == -1)
      {
        if (feof (stdin))
          break;
    
        error (EXIT_FAILURE, errno, _("input error"));
      }
    
      if (line[strlen (line) - 1] == '\n')
        line[strlen (line) - 1] = '\0';
      ....
    }

    Немного удивлены? Бывает. Новая уязвимость, соответствующий идентификатор CVE: CVE-2016-6262.

    Предупреждение PVS-Studio: V1010 CWE-20 Unchecked tainted data is used in index: 'strlen(line)'.

    С очередной попытки проблему поправили, добавив проверку длины входной строки:

    if (strlen (line) > 0)
      if (line[strlen (line) - 1] == '\n')
        line[strlen (line) - 1] = '\0';

    Давайте взглянем на даты. Коммит, 'закрывающий' CVE-2015-8948 – 10.08.2015. Коммит, закрывающий CVE-2016-62-62 – 14.01.2016. То есть разница между приведёнными исправлениями составляет 5 месяцев! Вот тут-то вспоминаешь про такое преимущество статического анализа, как обнаружение ошибок на ранних этапах написания кода…

    Статический анализ и безопасность


    Дальше примеров кода не будет, вместо этого – статистика и рассуждения. В этом разделе мнение автора может не совпадать с мнением читателя куда больше, чем раньше в этой статье.

    Примечание. Рекомендую ознакомиться с другой статьёй на схожую тему – "Как PVS-Studio может помочь в поиске уязвимостей?". Там есть интересные примеры уязвимостей, которые выглядят как простые ошибки. Дополнительно, в той статье я немного рассказал о терминологии и почему статический анализ – must have, если вас волнует тема безопасности.

    Давайте посмотрим на статистику по количеству обнаруженных за последние 10 лет уязвимостей, чтобы оценить ситуацию. Данные я взял с сайта CVE Details.

    Picture 2



    Интересная вырисовывается ситуация. До 2014 года количество зафиксированных CVE не превышало отметки в 6000 единиц, а начиная с – уже не опускалось ниже. Но самым интересным здесь, конечно, выглядит статистика за 2017 год – абсолютный лидер (14714 единиц). Что касается текущего – 2018 – года, он хоть ещё не закончился, но уже бьёт рекорды – 15310 единиц.

    Значит ли это, что весь новый софт дырявый как решето? Не думаю, и вот почему:

    • Повышенный интерес к теме уязвимостей. Наверняка, даже если вы не очень близки непосредственно к теме безопасности, неоднократно приходилось натыкаться на статьи, заметки, доклады и видео, посвящённые теме безопасности. Иначе говоря, создался некий «хайп». Плохо ли это? Пожалуй, нет. В конечном итоге всё сводится к тому, что разработчики больше заботятся о безопасности приложений, что хорошо.
    • Увеличение количества разрабатываемых приложений. Больше кода – больше вероятность возникновения какой-нибудь уязвимости, которая пополнит собой статистику.
    • Улучшение инструментов поиска уязвимостей и обеспечения качества кода. Больше спрос -> больше предложение. Анализаторы, фаззеры и прочие инструменты становятся всё более совершенными, что играет на руку тем, кто хочется искать уязвимости (вне зависимости от того, с какой стороны баррикад они находятся).

    Так что складывающуюся тенденцию нельзя назвать исключительно отрицательной – издатели больше заботятся об информационной безопасности, инструменты поиска проблем совершенствуются, а всё это, несомненно, позитивно.

    Значит ли это, что можно расслабиться и «не париться»? Думаю, нет. Если вас беспокоит тема безопасности ваших приложений, следует принимать как можно больше мер по обеспечению безопасности. Особенно это актуально, если исходный код находится в открытом доступе, так как он:

    • более подвержен встраиванию уязвимостей со стороны;
    • более подвержен 'зондированию' теми 'джентльменами', которых интересуют дыры в вашем приложении с целью их эксплуатации. Хотя и доброжелатели в таком случае помочь вам смогут больше.

    Не хочу сказать, что не нужно переводить свои проекты под open source. Просто следует не забывать о надлежащих мерах контроля качества / безопасности.

    Является ли статический анализ такой дополнительной мерой? Да. Статический анализ хорошо справляется с обнаружением потенциальных уязвимостей, которые в дальнейшем могут стать вполне реальными.

    Мне кажется (допускаю, что ошибаюсь), что многие считают уязвимости явлением достаточно высокоуровневым. И да, и нет. Проблемы в коде, которые кажутся простыми ошибками программирования, тоже вполне могут быть серьёзными уязвимостями. Опять же, некоторые примеры таких уязвимостей приведены в упоминавшейся ранее статье. Не следует недооценивать 'простые' ошибки.

    Заключение


    Не забывайте, что входные данные могут иметь нулевую длину, и это тоже нужно учитывать.

    Выводы по поводу того, является ли вся шумиха с уязвимостями просто шумихой, или же проблема существует, делайте сами.

    Со своей стороны, разве что предложу попробовать на своём проекте PVS-Studio, если вы ещё этого не сделали.

    Всех благ!



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Sergey Vasiliev. Shoot yourself in the foot when handling input data
    • +33
    • 8,3k
    • 8
    PVS-Studio
    421,00
    Static Code Analysis for C, C++, C# and Java
    Поделиться публикацией

    Комментарии 8

      +4
      Чем примечательна данная статья?
      • Нас давно просили продемонстрировать как «завалить» программу, раз мы знаем, где
        в коде ошибка.
      • Демонстрация настроящих уязвимостей.
      • Наконец видео в статье, а не только примеры кода.

      Похоже взята новая планка уровня статей про проверку кода. Кто готов хотя бы повторить?
        +1

        С удовольствием читаю ваши статьи. Хочется подкинуть идею где ещё можно найти прекрасных тем для статей. В Python есть множество популярных библиотек которые используют С и С++. Я думаю на них можно написать ни одну интересную статью.

          +2
          Спасибо, действительно хорошее место.
            +1
            С удовольствием читаю ваши статьи.

            Спасибо, приятно слышать!

            Хочется подкинуть идею где ещё можно найти прекрасных тем для статей. В Python есть множество популярных библиотек которые используют С и С++.

            Спасибо за наводку. Если будет желание и конкретные проекты для проверки — можете отписать о них (через PR, например) в соответствующем проекте на GitHub.

            Там уже такое количество проектов набралось, правда — ух! :)
            0

            С удовольствием читаю Ваши статьи. Хочу подкинуть идею где ещё можно найти ещё прекрасных тем для статей. В Python есть множество популярных библиотек которые используют С и С++. Я думаю на них можно написать не одну интересную статью.

              +1
              Как админ, шкурно хочу упросить вас сделать обзор по KVM/Qemu.
                +1
                Вы уверены что хотите знать что там?
                  +1
                  Если честно, то боюсь, НО…
                  1) лучше всё-таки знать, так как активно используем
                  2) немного надежды что такой легендарный программист как Фабрис Беллар (фр. Fabrice Bellard) вероятно высоко поднял планку качества

              Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.

              Самое читаемое