После обширного тестирования GitHub открыл в открытом доступе функцию сканирования кода на уязвимости. Любой желающий может запустить сканер на собственном репозитории и найти уязвимости до того, как они пойдут в продакшн. Сканер действует для репозиториев на C, C++, C#, JavaScript, TypeScript, Python и Go.

Сканер основан на технологии CodeQL, которую разработала компания Semmle, купленная GitHub в прошлом году. CodeQL считается первым в мире сканером на уязвимости. В мае 2020 года началось бета-тестирование на GitHub. Теперь функция доступна для всех.

Как включить

Сканирование запускается со вкладки Security в репозитории.



Там нажимаем Set up code scanning.



В следующем окне нужно выбрать workflow, который мы хотим использовать для сканирования. Дело в том, что CodeQL поддерживает подключение сторонних движков. Для стандартного движка выбираем Анализ CodeQL.



В принципе, данный workflow можно настроить: включить сканирование по расписанию, сканирование на каждый push или пул-реквест, использовать собственный конфигурационный файл, запустить дополнительные поисковые запросы при сканировании.

Затем нажимаем кнопку Start commit и пишем название для нового коммита.



Выбираем коммит в главную ветку или создать новую ветку и запустить пул-реквест.



Это всё. В конце нажимаем кнопку Commit new file или Propose new file.

После указания коммита сканер уязвимостей будет анализировать ваш код в соответствии с частотой, указанной в файле workflow.

После активации CodeQL можно смотреть результаты и изменять параметры сканирования.

Движок CodeQL

Движок CodeQL ищет потенциальные уязвимости по словарю из более 2000 запросов. Словарь составлен GitHub и сообществом пользователей, которые тестировали систему. Эта база будет постоянно пополняться, да и каждый может дополнить её в индивидуальном порядке, просто отредактировав конфигурационный файл.

Инструмент сканирования построен по стандарту статического анализа кода SARIF (OASIS Static Analysis Results Interchange Format) и поддерживает подключение сторонних движков, которые будут работать в едином интерфейсе. Также поддерживается экспорт результатов через единые API.

С момента представления в мае 2020 года отсканировано более 12000 репозиториев (всего 1,4млн проходов) и найдено более 20000 проблем безопасности, включая уязвимости удалённого исполнения кода (RCE), SQL-инъекции и межсайтовый скриптинг (XSS).

Разработчики и мейнтейнеры исправили 72% найденных уязвимостей в течение 30 дней после их обнаружений, до слияния кода с основной веткой. Это хороший результат, потому что по статистике менее 30% найденных уязвимостей исправляются в течение месяца после обнаружения.

По итогам бета-тестирования в опенсорсный словарь запросов сделано 132 коммита от сообщества. Чтобы пользователи GitHub могли запускать сторонние инструменты, заключены соглашения с более чем десятком разработчиков систем безопасности и опенсорсных инструментов для статического анализа, сканирования контейнеров и валидации инфраструктуры как кода (Infrastructure-as-Code; IaC) это подход для управления и описания инфраструктуры через конфигурационные файлы, а не через ручное редактирование конфигураций на серверах или интерактивное взаимодействие.

Дополнительно к поиску уязвимостей GitHub также сотрудничает с 24 сторонними сервис-провайдерами, чтобы находить в коде их секреты, которые нельзя публиковать в открытом виде, такие как ключи доступа. Среди партнёров AWS, Google Cloud, Azure, Dropbox, Slack, Discord, npm, Stripe и Twilio, Сканирование на секреты происходит автоматически и в публичных, и в приватных репозиториях.



Сканирование кода является бесплатным для публичных репозиториев и входит в пакет Advanced Security для GitHub Enterprise (то есть это платная услуга). Некоторые экзотические опции (список разрешённых IP-адресов, поддержка SAML, LDAP и др.) доступны только в платном варианте.

Впрочем, в эту бочку мёда нужно добавить ложку дёгтя. Некоторые авторы опенсорсных программ жалуются (1, 2), что сканирование даёт слишком много ложноположительных срабатываний.


В теории автоматическая проверка всех репозиториев это хорошее дело, но на практике не очень приятно постоянно отвлекаться на сообщения о ложных уязвимостях, особенно в dev-репозиториях или устаревших архивах, которые никогда не пойдут в продакшн. Такое очень быстро надоедает. Некоторые авторы говорят, что в их собственном коде большинство уязвимостей на самом деле шум или не применимо в конкретном случае.

То есть сканер GitHub может вызвать все симптомы состояния, известного как усталость от безопасности (security fatigue). Подробнее об этом состоянии см. в научной статье (doi: 10.1109/MITP.2016.84). Там говорится, что это состояние у человека подкрепляет его нежелание следовать рекомендациям по безопасности и влияет на общий анализ выгоды и затрат.

Я помню выход PHP7: появились strict types, скалярные type hint-ы.

Мы начали двигаться в сторону языка со статической типизацией, но типизация не ушла в статику. Концептуально все осталась прежним мы запускаем программу и только в runtime узнаем, что где-то есть неправильный тип. Даже если мы везде явно проставим типы, все ошибки мы не поймаем и можем больно упасть в продакшене.

Всем привет! Это частичная расшифровка моего подкаста Между скобок разговоров обо всем, что связано с разработкой на PHP. В гостях у меня Валентин Удальцов, автор телеграм-канала Пых, который регулярно пишет о плюсах статического анализа.

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

Если вы решите углубиться в тему, советую эти видео:

• Обзор инструментов для аудита кода от Александра Новикова
• И мой недавний доклад, в котором я рассказал о первых результатах внедрения статического анализа в команде

Поехали!

Сергей Жук, Skyeng: Что вообще анализируют статические анализаторы? Они же не только про типы, да?

Валентин Удальцов, Пых: Статический анализ выходит далеко за пределы декларации типов в PHP. Типизация может быть очень многогранна: скажем, это может быть работа с иммутабельность и чистотой. То есть, мы можем гарантировать, что объект определенного класса не меняется в процессе его использования, на нем нет setter-ов.

Мы также можем следить за чистотой функции: смотреть, что она не обращается к IO.

В Psalm еще недавно добавили taint analysis: мы можем проследить данные от источника из внешней среды до какого-то уровня вглубь и понять, что все данные были так или иначе провалидированы. Тогда, с некоторой долей вероятности, у нас нет проблем с безопасностью. Вот еще одна задача, которую может решать статический анализ.

Сергей Жук, Skyeng: А ты вообще разбирался, как это работает? Если мой код не выполняется, получается, он парсится в абстрактное синтаксическое дерево и...

Валентин Удальцов, Пых: Есть статические анализаторы, которые используют autoloading композеровский те же PHPStan и Psalm. Phan по-моему, частично использует runtime, но дальше сам анализ происходит без выполнения.

В этом вся суть. В основе большинства статических анализаторов, речь не только про тройку, что я назвал, используется парсер, который написан на PHP Никитой Поповым. При помощи этого парсера код разбивается на токены, и мы можем анализировать вызовы, получать набор expression-ов, statement-ов, делать какие-то выводы о том, как данные протекают в процессе выполнения в этих участках кода.

Есть два этапа. На первом Psalm проходит весь код, который ты указал ему явно полностью. Традиционно это папка src и любые другие, которые ты перечислил. Он также заглядывает в vendor, если твой код использует вендорные классы. Это просто сбор данных: он все кэширует, трансформируя данные парсера Попова в некоторые свои сущности.

Потом, при следующих проходах, первый этап повторяется только для тех файлов и зависимостей, которые поменялись. За счёт этого Psalm работает достаточно быстро.

Второй этап это анализ. По умолчанию он проходит только в измененных файлах. Но можно форсировать полный анализ: тогда Psalm поднимет из кеша подготовленные на первом этапе данные и проведет рекурсивный анализ всего и вся.

Сергей Жук, Skyeng: Ты постоянно упоминаешь Psalm а почему не другие?

Валентин Удальцов, Пых: Да все просто. Когда я вообще что-то выбирал, то договорился на работе в Happy Inc., что начинаю новый проект и мы сразу включаем всё на максимум. Посмотрел, что чаще упоминают, сделал базовое сравнение фич. Phan не использовал composer autoloading, насколько помню. В PHPStan было больше плагинов. В Psalm была иммутабельность, еще какие-то вещи, частые релизы, сообщество, а человек, который его развивает, на оплачиваемой должности. Все это подкупило.

Выбор я сделал и не слежу, как развиваются альтернативы. Просто времени нет. Но Psalm до сих пор часто удивляет: скажем, с новым релизом может появить фича, про которую я знать не знал, что это можно проверять статически.

Бывает и так, что выйдет новый релиз, а там какая-то регрессия или на что-то не написан тест.

Естественно, приходится такое suppress-ить, писать issue. Мой лайфхак: если вы нашли проблему в любом пакете, который ставите, напишите todo, создайте issue, а в todo впишите ссылку на issue. Когда issue закроют, по его url вы найдете все места в коде, где нужно убрать suppress.

С Psalm я так и поступаю: не откатываю версию, я просто пишу suppress-ы и обязательно делаю тикет.

Сергей Жук, Skyeng: А как подсадить на статанализ команду? Люди ждут, что им сейчас всякие хитрые баги найдут, а в реальности все немного иначе.

Валентин Удальцов, Пых: У меня нет опыта внедрения Psalm на большом проекте с кучей людей. Этот момент я признаю. Мне помогло то, что я начинал писать проект один, хотел по полной обмазываться статическим анализом, и сделал это. А когда начали подключаться люди с других проектов, они были поставлены в существующие рамки. Конечно, поначалу я объяснял и показывал, но у нас небольшой техотдел. Так что не сказал бы, что адаптация других ребят заняла много времени.

Для тех, у кого много людей и старая кодовая база, в инструментах статического анализа есть такая история, как baseline.

Когда ты добавляешь инструмент в старый проект, там найдется миллион ошибок. Но ты говоришь анализатору: мы признаем, что это есть, но пока не обращаем на это внимание а вот когда мы пишем новое, рассказывай, где проблемы. Это baseline и постепенно, относительно него, разработчики учатся писать код с учетом статического анализа. При этом в Psalm есть несколько уровней, то есть инструмент максимально заточен, чтобы внедрять его постепенно.

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

Сергей Жук, Skyeng: Бывает, анализаторы ругаются на неверные PHP-доки, хотя сам код работает правильно. Да и ты говорил про баги, которые помечаешь todo-шками Есть ли тогда смысл добавлять статанализ наравне с тестами в CI?

Валентин Удальцов, Пых: Добавлять надо, иначе теряется смысл. Идея типизации в том, чтобы снизить энтропию, повысить детерминированность того, что мы пишем. А богатая типизация позволяет писать меньше тестов. Если мы знаем, что где-то есть тип, который возвращает в этом случае null, а в этом string, если мы точно знаем, в каких случаях что возвращается, то не понадобится писать тест, который это проверяет.

Статический анализ это полноценный этап CI, который пробегает до тестов.

Если у тебя типы неверные, что там тестировать? Ты в любом случае накосячил.

Что касается багов в Psalm, обычно это минорные вещи, не самая большая проблема. Если есть реальная проблема, мы формируем issue и костыльком в виде suppress или подсказки с todo-шкой это фиксим. При этом бывают нюансы, которые ты сразу не понимаешь, делаешь issue, а тебе отвечают: Да ты просто здесь вот это не так указал. И скидывают как правильно.

Сергей Жук, Skyeng: Ок, но для таких инструментов свой синтаксис надо учить. И твой код им обрастет. Как ты думаешь, в будущем это всё мигрирует в сам PHP?

Валентин Удальцов, Пых: Я смотрю на свой текущий проект. Он написан как бы не на PHP, а на PHP плюс Psalm. Но это всё-таки далеко от того, чтобы быть прям совсем другим языком. Это не как Haskell и PHP, например.

Что касается будущего, трудно сказать. Если код не типизирован, вероятность ошибки выше. Пока PHP как бы на двух стульях сидит: с одной стороны, позволяет mixed (и это иногда удобно), а с другой, мы так или иначе стремимся к типизации.

Сергей Жук, Skyeng: Ок, а что бы ты сказал человеку, который сейчас открыл IDE, создал новый проект и думает, добавлять статанализ или ну его?

Валентин Удальцов, Пых: Бизнес-логику, естественно, никакой статический анализ не проверит. А вот типы и все эти совершенные тупые ошибки снижаются процентов на 80. Да, в начале будет сложнее. Но в долгосрочной перспективе это профит. Статический анализ передвигает исправление багов ближе к началу разработки, а это снижает издержки на поддержку проекта.

Ведь всех бесят ошибки с продакшена, когда вместо expected string пришел int, null где-то появился или undefined index. Поэтому разработчик может взвесить все плюсы и минусы, понять, что благодаря статическому анализу вот такие совершенно глупые ошибки исключаются. А еще есть приятный бонус в виде дженериков.

Сергей Жук, Skyeng: А всё же есть какие-то минусы, каких-то кейсы, где вообще не надо использовать стат анализ?

Валентин Удальцов, Пых: Если проект реально большой, а команда привыкла полагаться на runtime издержки на интеграцию могут быть запредельными. Но все равно я бы советовал попробовать baseline внедрить, посмотреть, что как будет. Многие вещи надо в несколько этапов внедрять.

А если вы распиливаете большой монолитный сервис, и по какой-то причине продолжаете на PHP все делать, стоит попробовать.

Это не только хайп последних лет. Это реально полезно. Если мы полагаемся на runtime, то обязательно наткнемся на сложную ситуацию, что и поправить безумно дорого, и клиент готов уйти, и всё-всё очень плохо.

p.s. Найти другие выпуски подкаста вы можете здесь.

За окном зима, год стремится к завершению, а значит, пришло время рассмотреть самые интересные ошибки, обнаруженные анализатором PVS-Studio в 2020 году.

Стоит отметить, что прошедший год ознаменовался большим количеством новых диагностических правил, срабатывания которых позволили им попасть в данный топ. Также мы продолжаем улучшать ядро анализатора и добавлять новые сценарии его использования, обо всём этом можно почитать в нашем блоге. Если вам интересны другие поддерживаемые нашим анализатором языки (C, C# и Java), обратите внимание на статьи моих коллег. Теперь же перейдём непосредственно к самым запомнившимся мне багам, найденным PVS-Studio за прошедший год.

Десятое место: Деление по модулю на единицу

V1063 The modulo by 1 operation is meaningless. The result will always be zero. llvm-stress.cpp 631

void Act() override {  ....  // If the value type is a vector, and we allow vector select,  // then in 50% of the cases generate a vector select.  if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 1)) {    unsigned NumElem =        cast<FixedVectorType>(Val0->getType())->getNumElements();    CondTy = FixedVectorType::get(CondTy, NumElem);  }  ....}

Разработчик хотел получить случайное значение в диапазоне от 0 до 1, использовав деление по модулю. Однако операция вида X%1 всегда вернёт 0. В данном случае правильно было бы переписать условие следующим образом:

if (isa<FixedVectorType>(Val0->getType()) && (getRandom() % 2))

Эта ошибка вошла в топ из статьи: "Проверка Clang 11 с помощью PVS-Studio".

Девятое место: Четыре проверки

На следующий участок кода PVS-Studio выдал четыре предупреждения:

• V560 A part of conditional expression is always true: x >= 0. editor.cpp 1137
• V560 A part of conditional expression is always true: y >= 0. editor.cpp 1137
• V560 A part of conditional expression is always true: x < 40. editor.cpp 1137
• V560 A part of conditional expression is always true: y < 30. editor.cpp 1137

int editorclass::at( int x, int y ){  if(x<0) return at(0,y);  if(y<0) return at(x,0);  if(x>=40) return at(39,y);  if(y>=30) return at(x,29);  if(x>=0 && y>=0 && x<40 && y<30)  {      return contents[x+(levx*40)+vmult[y+(levy*30)]];  }  return 0;}

Все предупреждения относятся к последнему if-выражению. Проблема в том, что все четыре проверки, которые в нём выполняются, всегда будут возвращать true. Не сказал бы, что это серьезная ошибка, но получилось довольно забавно. В общем, данные проверки являются избыточными и их можно убрать.

Эта ошибка вошла в топ из статьи: "VVVVVV??? VVVVVV!!!".

Восьмое место: delete вместо delete[]

V611 The memory was allocated using 'new T[]' operator but was released using the 'delete' operator. Consider inspecting this code. It's probably better to use 'delete [] poke_data;'. CCDDE.CPP 410

BOOL Send_Data_To_DDE_Server (char *data, int length, int packet_type){  ....  char *poke_data = new char [length + 2*sizeof(int)]; // <=  ....  if(DDE_Class->Poke_Server( .... ) == FALSE) {    CCDebugString("C&C95 - POKE failed!\n");    DDE_Class->Close_Poke_Connection();    delete poke_data;                                  // <=    return (FALSE);  }  DDE_Class->Close_Poke_Connection();  delete poke_data;                                    // <=  return (TRUE);}

Анализатор обнаружил ошибку, связанную с тем, что память выделена и освобождена несовместимыми между собой способами. Для освобождения памяти, выделенной под массив, следует использовать оператор delete[], а не delete.

Эта ошибка вошла в топ из статьи: "Код игры Command & Conquer: баги из 90-х. Том второй".

Седьмое место: Выход за границу буфера

Рассмотрим функцию net_hostname_get, которая будет использоваться дальше.

#if defined(CONFIG_NET_HOSTNAME_ENABLE)const char *net_hostname_get(void);#elsestatic inline const char *net_hostname_get(void){  return "zephyr";}#endif

В данном случае при препроцессировании выбирался вариант, относящийся к ветке #else. То есть, в препроцессированном файле функция реализуется так:

static inline const char *net_hostname_get(void){  return "zephyr";}

Функция возвращает указатель на массив из 7 байт (учитываем терминальный ноль в конце строки).

Теперь рассмотрим код, приводящий к выходу за границу массива.

static int do_net_init(void){  ....  (void)memcpy(hostname, net_hostname_get(), MAX_HOSTNAME_LEN);  ....}

Предупреждение PVS-Studio: V512 [CWE-119] A call of the 'memcpy' function will lead to the 'net_hostname_get()' buffer becoming out of range. log_backend_net.c 114

После препроцессирования MAX_HOSTNAME_LEN раскрывается следующим образом:

(void)memcpy(hostname, net_hostname_get(),    sizeof("xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx:xxxx"));

Соответственно, при копировании данных возникает выход за границу строкового литерала. Как это скажется на выполнении программы предсказать сложно, так как это приводит к неопределённому поведению.

Эта ошибка вошла в топ из статьи: "Исследуем качество кода операционной системы Zephyr".

Шестое место: Что-то очень странное

static char *mntpt_prepare(char *mntpt){  char *cpy_mntpt;  cpy_mntpt = k_malloc(strlen(mntpt) + 1);  if (cpy_mntpt) {    ((u8_t *)mntpt)[strlen(mntpt)] = '\0';    memcpy(cpy_mntpt, mntpt, strlen(mntpt));  }  return cpy_mntpt;}

Предупреждение PVS-Studio: V575 [CWE-628] The 'memcpy' function doesn't copy the whole string. Use 'strcpy / strcpy_s' function to preserve terminal null. shell.c 427

Кто-то пытался сделать аналог функции strdup, но у него это не получилось.

Начнём с предупреждения анализатора. Он сообщает, что функция memcpy копирует строчку, но не скопирует терминальный ноль, и это очень подозрительно.

Кажется, что этот терминальный 0 копируется здесь:

((u8_t *)mntpt)[strlen(mntpt)] = '\0';

Но нет! Здесь опечатка, из-за которой терминальный ноль копируется сам в себя! Обратите внимание, что запись происходит в массив mntpt, а не в cpy_mntpt. В итоге функция mntpt_prepare возвращает строку, незавершенную терминальным нулём.

На самом деле программист хотел написать так:

((u8_t *)cpy_mntpt)[strlen(mntpt)] = '\0';

Однако всё равно не понятно, зачем сделано так сложно! Этот код можно упростить до следующего варианта:

static char *mntpt_prepare(char *mntpt){  char *cpy_mntpt;  cpy_mntpt = k_malloc(strlen(mntpt) + 1);  if (cpy_mntpt) {    strcpy(cpy_mntpt, mntpt);  }  return cpy_mntpt;}

Эта ошибка вошла в топ из вышеупомянутой статьи: "Исследуем качество кода операционной системы Zephyr".

Пятое место: Неправильная защита от переполнения

V547 [CWE-570] Expression 'rel_wait < 0' is always false. Unsigned type value is never < 0. os_thread_windows.c 359

static DWORDget_rel_wait(const struct timespec *abstime){  struct __timeb64 t;  _ftime64_s(&t);  time_t now_ms = t.time * 1000 + t.millitm;  time_t ms = (time_t)(abstime->tv_sec * 1000 +    abstime->tv_nsec / 1000000);  DWORD rel_wait = (DWORD)(ms - now_ms);  return rel_wait < 0 ? 0 : rel_wait;}

В данном случае переменная rel_wait имеет беззнаковый тип DWORD. А значит, сравнение rel_wait < 0 не имеет смысла, так как результатом всегда является истина.

Сама по себе ошибка не очень интересная. Зато интересно вышло с тем, как её попробовали исправить. Получилось, что изменения не исправили, а лишь упростили код. Подробнее про эту историю можно прочитать в статье моего коллеги: "Почему PVS-Studio не предлагает автоматические правки кода".

Ошибка же вошла в топ из статьи: "Статический анализ кода коллекции библиотек PMDK от Intel и ошибки, которые не ошибки".

Четвёртое место: Не пиши в std, брат

V1061 Extending the 'std' namespace may result in undefined behavior. sized_iterator.hh 210

// Dirty hack because g++ 4.6 at least wants// to do a bunch of copy operations.namespace std {inline void iter_swap(util::SizedIterator first,                      util::SizedIterator second){  util::swap(*first, *second);}} // namespace std

В статье, из которой взято срабатывание: "Анализ кода проекта DeepSpeech или почему не стоит писать в namespace std" подробно описано, почему не стоит поступать подобным образом.

Третье место: Скроллбар, который не смог

V501. There are identical sub-expressions to the left and to the right of the '-' operator: bufferHeight bufferHeight TermControl.cpp 592

bool TermControl::_InitializeTerminal(){  ....  auto bottom = _terminal->GetViewport().BottomExclusive();  auto bufferHeight = bottom;  ScrollBar().Maximum(bufferHeight - bufferHeight);  ScrollBar().Minimum(0);  ScrollBar().Value(0);  ScrollBar().ViewportSize(bufferHeight);  ....}

Это, что называется, "срабатывание с историей". В данном случае из-за ошибки не работал скроллбар в Windows Terminal. По мотивам данного бага написана целая статья, в которой мой коллега провёл исследование и разобрался почему так случилось. Заинтересовались? Вот она: "Скроллбар, который не смог".

Второе место: перепутали радиус и высоту

И опять речь пойдёт о нескольких предупреждениях анализатора:

• V764 Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 791
• V764 Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 833
• V764 Possible incorrect order of arguments passed to 'CreateWheel' function: 'height' and 'radius'. StandardJoints.cpp 884

Привожу вызовы функции:

NewtonBody* const wheel = CreateWheel (scene, origin, height, radius);

А так выглядит её объявление:

static NewtonBody* CreateWheel (DemoEntityManager* const scene,  const dVector& location, dFloat radius, dFloat height)

При вызовах функций аргументы были перепутаны местами.

Эта ошибка вошла в топ из статьи: "Повторная проверка Newton Game Dynamics статическим анализатором PVS-Studio".

Первое место: Затирание результата

V519 The 'color_name' variable is assigned values twice successively. Perhaps this is a mistake. Check lines: 621, 627. string.cpp 627

static bool parseNamedColorString(const std::string &value,                                  video::SColor &color){  std::string color_name;  std::string alpha_string;  size_t alpha_pos = value.find('#');  if (alpha_pos != std::string::npos) {    color_name = value.substr(0, alpha_pos);    alpha_string = value.substr(alpha_pos + 1);  } else {    color_name = value;  }  color_name = lowercase(value); // <=  std::map<const std::string, unsigned>::const_iterator it;  it = named_colors.colors.find(color_name);  if (it == named_colors.colors.end())    return false;  ....}

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

Однако затем в функции lowercase() в нижний регистр переводится не сама полученная строка, а исходный аргумент функции. В результате мы просто потеряем цвет, который должна была вернуть parseNamedColorString().

color_name = lowercase(color_name);

Эта ошибка вошла в топ из статьи: "PVS-Studio: Анализ pull request-ов в Azure DevOps при помощи self-hosted агентов".

Заключение

За прошедший год мы нашли много ошибок в open source проектах. Это были привычные ошибки copy-paste, ошибки в константах, утечки памяти и множество других проблем. Наш анализатор не стоит на месте и в топе присутствует несколько срабатываний новых диагностик, написанных в этом году.

Надеюсь, вам понравились собранные ошибки. Лично мне они показались достаточно интересными. Но, конечно, ваше видение может отличаться от моего, поэтому вы можете составить свой "Tоп 10...", почитав статьи из нашего блога или посмотрев список ошибок, найденных PVS-Studio в open source проектах.

Также предлагаю вашему вниманию статьи с топ 10 C++ ошибок прошлых лет: 2016, 2017, 2018, 2019.


Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: Vladislav Stolyarov. Top 10 Bugs Found in C++ Projects in 2020.