Недавно сайт Pinguem.ru совместно с командой PVS-Studio устраивали конкурс, в котором программистам было необходимо в течение месяца использовать статический анализатор PVS-Studio для нахождения и исправления ошибок в коде open-source проектов. Благодаря их стараниям, программы в мире стали чуточку безопаснее и надежнее. В статье мы рассмотрим парочку наиболее интересных ошибок, которые были найдены при помощи PVS-Studio.
Конкурс проходил с 23 октября по 27 ноября 2017 года в два этапа для русскоязычной аудитории. На первом этапе конкурсантам было необходимо отправить как можно больше Pull Request'ов авторам проектов. Второй этап был несколько сложнее: нужно было найти ошибку и описать последовательность действий, при которых она бы себя проявила в работе приложения. Лучше всех с заданиями справился Николай Шалакин и стал победителем конкурса. Поздравляем его!
За время проведения конкурса было отправлено немало хороших Pull Request'ов, желающие могут ознакомиться с ними по этой ссылке. Мы же предлагаем рассмотреть наиболее интересные ошибки, найденные конкурсантами на втором этапе.
Многие ли из Вас используют QtCreator для программирования на Python? Как и многие IDE, он тоже подсвечивает некоторые встроенные функции и объекты. Возьмем QtCreator 4.4.1 и напишем несколько зарезервированных слов:
Что же такое? Почему встроенные функции oct и chr не подсвечиваются? Взглянем на код:
// List of python built-in functions and objects
static const QSet<QString> builtins = {
"range", "xrange", "int", "float", "long", "hex", "oct" "chr", "ord",
"len", "abs", "None", "True", "False"
};Функции объявлены, они должны подсвечиваться. И здесь помогает PVS-Studio:
V653 A suspicious string consisting of two parts is used for initialization. It is possible that a comma is missing. Consider inspecting this literal: "oct" "chr". pythonscanner.cpp 205
Действительно, между литералами "oct" и "chr" забыли поставить запятую, поэтому два литерала слились в один "octchr", и именно он будет подсвечиваться средой разработки:
Здесь можно ознакомиться с Pull Request'ом на исправление.
Вы работаете над проектом ConEmu и в отладочной версии решили проверить работу некоторых настроек (нажмите на анимацию для увеличения):
Давайте взглянем на код, почему вылетает предупреждение "ListBox was not processed":
INT_PTR CSetPgViews::OnComboBox(HWND hDlg, WORD nCtrlId, WORD code)
{
switch (code)
{
....
case CBN_SELCHANGE:
{
....
UINT val;
INT_PTR nSel = SendDlgItemMessage(hDlg,
nCtrlId,
CB_GETCURSEL,
0,
0);
switch (nCtrlId)
{
....
case tThumbMaxZoom:
gpSet->ThSet.nMaxZoom = max(100,((nSel+1)*100));
default:
_ASSERTE(FALSE && "ListBox was not processed");
}
}
}
}Из-за пропущенного break управление перейдет к ветке default после того, как отработают выражения в ветке tThumbMaxZoom. Об этом предупреждает PVS-Studio:
V796 It is possible that 'break' statement is missing in switch statement. setpgviews.cpp 183
Здесь можно ознакомиться с Pull Request'ом на исправление.
Довольно интересный проект, особенно полезный для геймеров. При нажатии на кнопку Pause программа приостанавливает работу окна, находящегося на переднем плане:
Можно переназначить кнопку приостановки/возобновления на другую при помощи файла settings.txt:
Если ввести код ключа не менее, чем 20 символов, и не более, чем 30 символов, это приведет к порче стека (нажмите на анимацию для увеличения):
Разберемся, почему это происходит. Нас интересует функция LoadPauseKeyFromSettingsFile:
int LoadPauseKeyFromSettingsFile(_In_ wchar_t* Filename)
{
HANDLE FileHandle = CreateFile(Filename,
GENERIC_READ,
FILE_SHARE_READ,
NULL,
OPEN_EXISTING,
FILE_ATTRIBUTE_NORMAL,
NULL);
if (FileHandle == INVALID_HANDLE_VALUE)
{
goto Default;
}
char KeyLine[32] = { 0 };
char Buffer[2] = { 0 };
DWORD ByteRead = 0;
do
{
if (!ReadFile(FileHandle, Buffer, 1, &ByteRead, NULL))
{
goto Default;
}
if (Buffer[0] == '\r' || Buffer[0] == '\n')
{
break;
}
size_t Length = strlen(KeyLine);
if (Length > 30) // <=
{
goto Default;
}
KeyLine[Length] = Buffer[0];
memset(Buffer, 0, sizeof(Buffer));
} while (ByteRead == 1);
if (!StringStartsWith_AI(KeyLine, "KEY="))
{
goto Default;
}
char KeyNumberAsString[16] = { 0 }; // <=
for (DWORD Counter = 4; Counter < strlen(KeyLine); Counter++) // <=
{
KeyNumberAsString[Counter - 4] = KeyLine[Counter];
}
....
Default:
if (FileHandle != INVALID_HANDLE_VALUE && FileHandle != NULL)
{
CloseHandle(FileHandle);
}
return(0x13);
}В цикле считывается побайтово первая строка. Если она превышает длину в 30 символов, то выполнение переходит по метке Default, при этом освобождается ресурс и возвращается символ с кодом 0x13. Если чтение происходит успешно, и первая строка начинается с "KEY=", то происходит копирование подстроки после символа "=" в 16-байтовый буфер KeyNumberAsString. При вводе ключа от 20 до 30 символов произойдет переполнение буфера. Об этом предупреждает PVS-Studio:
V557 Array overrun is possible. The value of 'Counter - 4' index could reach 26. main.cpp 146
Здесь можно ознакомиться с Pull Request'ом на исправление.
В этом проекте был найден баг, связанный с сортировкой избранных вкладок (нажмите на анимацию для увеличения):
Посмотрим на код сортировки:
int CALLBACK SortByName(const NBookmarkHelper::variantBookmark_t
BookmarkItem1,
const NBookmarkHelper::variantBookmark_t
BookmarkItem2)
{
if ( BookmarkItem1.type() == typeid(CBookmarkFolder)
&& BookmarkItem2.type() == typeid(CBookmarkFolder))
{
const CBookmarkFolder &BookmarkFolder1 =
boost::get<CBookmarkFolder>(BookmarkItem1);
const CBookmarkFolder &BookmarkFolder2 =
boost::get<CBookmarkFolder>(BookmarkItem2);
return BookmarkFolder1.GetName()
.compare(BookmarkFolder2.GetName());
}
else
{
const CBookmark &Bookmark1 =
boost::get<CBookmark>(BookmarkItem1);
const CBookmark &Bookmark2 =
boost::get<CBookmark>(BookmarkItem1);
return Bookmark1.GetName().compare(Bookmark2.GetName());
}
}В ветке else ошиблись и дважды использовали BookmarkItem1, вместо BookmarkItem2. Об этой ошибке и предупреждает PVS-Studio:
Здесь можно ознакомиться с Pull Request'ом на исправление.
Команда PVS-Studio выражает огромную благодарность всем программистам, принявшим участие в конкурсе. Вы проделали огромную работу в устранении багов в open-source проектах, сделав их надежнее, безопаснее и лучше. В будущем мы, возможно, проведем подобный конкурс и для англоязычной аудитории.
Мы также предлагаем всем остальным скачать и попробовать анализатор PVS-Studio, он прост в обращении и может быть Вам полезен.