Вебинар: ГОСТ Р 71207–2024 — Статический анализ программного обеспечения. Процессы - 13.09
Ни один процесс разработки программы не проходит без отладки. Современные IDE обеспечивают программиста встроенным отладчиком. Однако бывают ситуации, когда использование IDE для отладки избыточно или невозможно, и тогда на помощь приходят автономные отладчики, один из которых — x64dbg.
x64dbg — это отладчик для 32-битных и 64-битных версий Windows с открытым исходным кодом, предлагающий "интуитивный и знакомый, пусть и обновлённый" пользовательский интерфейс. Внешне он напоминает OllyDbg, но с подтянутым оформлением и расширенным функционалом.
У этого отладчика нет стабильных релизов. Вместо них примерно раз в месяц публикуются сборки-снапшоты. Параллельно идёт разработка кроссплатформенной версии.
В какой ситуации может понадобиться автономный отладчик? Например, Visual Studio 2022 по умолчанию собирает исполняемые файлы с поддержкой Windows Vista, но установить её на Windows Vista и использовать удалённую отладку не получится из-за неподдерживаемой операционной системы — требуется минимум Windows 7.
Основным сценарием использования автономного отладчика всё же является изучение исполняемых файлов, к исходному коду и отладочным символам которого нет доступа. Антивирусные лаборатории определяют алгоритм работы вредоносного ПО, чтобы описать его в своей базе знаний, и затем добавляют его определение в антивирусные базы. Различные сообщества по возрождению старых сетевых игр или программ не убирают руки от отладчика, чтобы воссоздать их серверную сторону. И не только старых — даже под относительно недавние сетевые игры выходили эмуляторы сервера. Но насколько хорошо сделан швейцарский нож реверс-инженера? Самое время запустить анализатор PVS-Studio.
Можно загрузить PVS-Studio, перейдя на эту страницу. Чтобы произвести анализ проекта, понадобится лицензия, триальную версию которой можно получить здесь. Процедура установки PVS-Studio не вызовет сложностей: каждый шаг сопровождается легко воспринимаемым пояснением, а при возникновении трудностей всегда поможет справка по быстрому запуску под Windows. Нужно две дополнительные интеграции с IDE: для Qt Creator (установщик распакует её в папку PVS-Studio) и Visual Studio. Актуальная на момент написания статьи версия анализатора — 7.31.
Отладчик состоит из двух компонентов: ядра и графического интерфейса. Первый компонент собирается в Visual Studio, второй — в Qt Creator. В вики проекта есть инструкция по сборке, и автор настоятельно рекомендует использовать определённые версии зависимостей. В этой части статьи мы рассмотрим ядро отладчика. Проверка осуществляется на основе коммита f518e50 ветки development.
Открываем файл решения x64dbg.sln и в первую очередь убираем сторонние библиотеки из проверки, чтобы не утонуть в шуме не связанных с кодом x64dbg срабатываний. Эти библиотеки перечислены в фильтре Third Party проекта x64dbg_dbg в разделе заголовочных файлов:
\dbghelp\
\DeviceNameResolver\
\jansson\
\LLVMDemangle\
\lz4\
\msdia\
\ntdll\
\TitanEngine\
\WinInet-Downloader\
\XEDParse\
Исключить папки из анализа можно через настройки плагина Visual Studio: Extensions > PVS-Studio > Options > Don't Check Files. Альтернативные способы управления списком исключённых путей описаны в нашей документации.
Готовьте ваши инсектициды, начинаем вытравливать баги из дебаггера! Без дебаггера, как и обещал.
V570 The 'mLastChar' variable is assigned to itself. lexer.cpp 149
class Lexer
{
....
private:
....
int mLastChar = ' ';
....
....
}
Lexer::Token Lexer::getToken()
{
....
//character literal
if(mLastChar == '\'')
{
std::string charLit;
while(true)
{
....
if(mLastChar == '\\') //escape sequence
{
nextChar();
if(mLastChar == EOF)
return reportError("unexpected end of file in character literal (2)");
if(mLastChar == '\r' || mLastChar == '\n')
return reportError("unexpected newline in character literal (2)");
if( mLastChar == '\'' || mLastChar == '\"'
|| mLastChar == '?' || mLastChar == '\\')
mLastChar = mLastChar; // <=
else if(mLastChar == 'a')
mLastChar = '\a';
....
}
....
}
....
}
}
В этой функции осуществляется экранирование определённых символов: переноса строки, обратного слеша, кавычек и других служебных знаков. Переменная mLastChar является членом класса Lexer и содержит в себе последний прочитанный литерал. Функция nextChar читает следующий символ и записывает его в mLastChar. Если значение mLastChar обновлено её вызовом, то зачем ей ещё раз присваивать то же самое? Аналогичное срабатывание:
V547 Expression '!expr' is always true. parser.cpp 118
uptr<Expr> Parser::ParseExpr()
{
return nullptr;
}
uptr<Return> Parser::ParseReturn()
{
if(CurToken.Token == Lexer::tok_return)
{
NextToken();
auto expr = ParseExpr();
if(!expr) // <=
{
ReportError("failed to parse Return (ParseExpr failed)");
return nullptr;
}
return make_uptr<Return>(move(expr));
}
return nullptr;
}
Какова вероятность, что из ничего появится что-то? Правдиво ли средневековое суеверие, что мыши сами по себе возникают из грязного белья? Научно доказано, что это неосуществимо. Как и получение других данных из nullptr.
На всякий случай также приведу здесь определение класса uptr, который на самом деле является обычным std::unique_ptr:
template<class T>
using uptr = unique_ptr<T>;
V560 A part of conditional expression is always false: !haveCurrValue. watch.cpp 61
....
currValue = val;
haveCurrValue = true;
if(getType() != WATCHVARTYPE::TYPE_INVALID)
{
switch(getWatchdogMode())
{
....
case WATCHDOGMODE::MODE_CHANGED:
if(currValue != origVal || !haveCurrValue) // <=
{
duint cip = GetContextDataEx(hActiveThread, UE_CIP);
dprintf(....);
watchdogTriggered = 1;
}
break;
case WATCHDOGMODE::MODE_UNCHANGED:
if(currValue == origVal || !haveCurrValue) // <=
{
duint cip = GetContextDataEx(hActiveThread, UE_CIP);
dprintf(....);
watchdogTriggered = 1;
}
break;
}
}
return val;
....
Как минимум, у нас случай copy-paste-ориентированного программирования, как максимум — создание никогда не модифицируемой переменной haveCurrValue до окончания блока switch-case.
Аналогичное срабатывание:
V783 Dereferencing of the invalid iterator might take place. LinearPass.cpp 130
void LinearPass::AnalyseOverlaps()
{
....
// Erase blocks marked for deletion
m_MainBlocks.erase(std::remove_if(
m_MainBlocks.begin(), m_MainBlocks.end(), [](BasicBlock & Elem)
{
return Elem.GetFlag(BASIC_BLOCK_FLAG_DELETE);
}));
....
}
Для удаления данных из массива разработчики нередко используют erase-remove идиому. Суть её состоит в следующем: элементы массива, выбранные функцией std::remove, будут переставлены в конец, а сама функция вернёт итератор, с которого функция std::erase начнёт их стирать. По замыслу автора этот вызов должен был удалить все элементы, у которых присутствует флаг удаления, о чём свидетельствует комментарий над вызовом m_MainBlocks.erase.
Этот код содержит две проблемы:
Попробуем исправить сразу обе проблемы. Для этого воспользуемся второй перегрузкой функции std::erase, которая принимает на вход позицию, с которой надо начать стирать элементы, и конечный элемент, после которого стирание будет завершено.
m_MainBlocks.erase(std::remove_if(
m_MainBlocks.begin(), m_MainBlocks.end(), [](BasicBlock & Elem)
{
return Elem.GetFlag(BASIC_BLOCK_FLAG_DELETE);
}), m_MainBlocks.end());
Таким образом, мы исправили удаление только одного элемента, а заодно и защитились от пустого списка на удаление. В документации чётко написано, что в таком случае ничего выполняться не будет.
V560 A part of conditional expression is always true: addr < _base + _size. cmd-undocumented.cpp 382
bool cbInstrVisualize(int argc, char* argv[])
{
if(IsArgumentsLessThan(argc, 3))
return false;
duint start;
duint maxaddr;
....
{
....
//initialize
Zydis zydis;
duint _base = start;
duint _size = maxaddr - start;
Memory<unsigned char*> _data(_size);
MemRead(_base, _data(), _size);
for(duint addr = start, fardest = 0; addr < maxaddr;)
{
....
//continue algorithm
const unsigned char* curData =
(addr >= _base && addr < _base + _size) // <=
? _data() + (addr - _base)
: nullptr;
if(zydis.Disassemble(addr, curData, MAX_DISASM_BUFFER))
{
if(addr + zydis.Size() > maxaddr)
break; //we went past the maximum allowed address
....
}
....
}
....
}
....
}
Переменной _base было присвоено значение переменной start. Это значит, что проверка максимального адреса уже есть в условии цикла (addr < maxaddr) и не имеет смысла в тернарном операторе при инициализации переменной curData. Понятно, что ничего не понятно. Следите за руками:
Чтобы получить maxaddr, нужно к start прибавить _size. Используя нехитрую математику, получаем выражение следующего вида:
maxaddr = start + _size
Таким образом, становится понятно, что условие цикла addr < maxaddr тождественно условию addr < _base + _size внутри цикла. Получается то же самое, но в более "полной" записи.
V1053 Calling the 'AddRef' virtual function in the constructor may lead to unexpected result at runtime. pdbdiafile.cpp 23
//Taken from: https://msdn.microsoft.com/en-us/library/ms752876(v=vs.85).aspx
class FileStream : public IStream
{
FileStream(HANDLE hFile)
{
AddRef(); // <=
_hFile = hFile;
}
....
public:
virtual ULONG STDMETHODCALLTYPE AddRef(void)
{
return (ULONG)InterlockedIncrement(&_refcount);
}
....
}
Вижу словосочетание "взято из". Смотрю на образец кода из Microsoft Learn. Вижу вместо вызова функции AddRef простое использование переменной _refcount:
class FileStream : public IStream
{
FileStream(HANDLE hFile)
{
_refcount = 1;
_hFile = hFile;
}
....
}
Чем чревато использование виртуальных функций в конструкторах или деструкторах? Чревато нарушением логики их вызова в наследуемом классе. В целом да, "скопировано правильно", но можно же было сразу вызвать функцию InterlockedIncrement со ссылкой на _refcount? Она даже системная!
V547 Expression '!bRedirectSupported' is always true. x64dbg_launcher.cpp 76
static BOOL isWowRedirectionSupported()
{
BOOL bRedirectSupported = FALSE;
_Wow64DisableRedirection = (LPFN_Wow64DisableWow64FsRedirection)
GetProcAddress(GetModuleHandle(TEXT("kernel32")),
"Wow64DisableWow64FsRedirection");
_Wow64RevertRedirection = (LPFN_Wow64RevertWow64FsRedirection)
GetProcAddress(GetModuleHandle(TEXT("kernel32")),
"Wow64RevertWow64FsRedirection");
if(!_Wow64DisableRedirection || !_Wow64RevertRedirection)
return bRedirectSupported;
else
return !bRedirectSupported; // <=
}
На первый взгляд срабатывание диагностики ощущается совершенно нелогичным. Долго всматривался в эти строки, пришлось даже сходить за глазными каплями. Если до этого я видел жестокое обращение с памятью строковых переменных, то теперь приходится видеть жертву стоматолога с немного нестандартным подходом к удалению больного зуба. Но, отбросив пугающие сравнительные обороты, это же отличный повод для рефакторинга! Статический анализатор сослужит хорошую службу в этом деле.
Функция GetProcAddress возвращает адрес экспортированной функции по её имени или порядковому номеру из модуля, либо NULL, если запрошенная функция не была найдена. Здесь совершенно не нужна ещё одна переменная, чтобы вернуть результат проверки поддержки перенаправления файловой системы для подсистемы WoW64. Если у нас отсутствует одна из функций, то возвращается по умолчанию FALSE. Поэтому всю функцию можно сократить буквально в три операции:
static BOOL isWowRedirectionSupported()
{
_Wow64DisableRedirection = (LPFN_Wow64DisableWow64FsRedirection)
GetProcAddress(GetModuleHandle(TEXT("kernel32")),
"Wow64DisableWow64FsRedirection");
_Wow64RevertRedirection = (LPFN_Wow64RevertWow64FsRedirection)
GetProcAddress(GetModuleHandle(TEXT("kernel32")),
"Wow64RevertWow64FsRedirection");
return !_Wow64DisableRedirection || !_Wow64RevertRedirection;
}
Но потом глаза поднимаются в документацию Microsoft Learn на описание функции Wow64RevertWow64FsRedirection:
This function should not be called without a previous call to the Wow64DisableWow64FsRedirection function.
Any data allocation on behalf of the Wow64DisableWow64FsRedirection function is cleaned up by this function.
Оказывается, одна функция без другой существовать не может, поэтому проверка поддержки перенаправления в корне неверная! Доработаем доработку, исправим в ней возвращаемое значение:
return _Wow64DisableRedirection && _Wow64RevertRedirection;
Теперь всё правильно: если нет хотя бы одной функции, то перенаправление не поддерживается.
V1003 The macro 'TITANGETDRX' is a dangerous expression. The parameter 'titantype' must be surrounded by parentheses. breakpoint.h 8
#define TITANGETDRX(titantype) UE_DR0 + ((titantype >> 8) & 0xF)
Непосредственно по коду я не нашёл случаев, где параметр titantype представлял собой какое-либо выражение. Только варианты с передачей одной переменной. Тем не менее, если кто-то оступится и забудет, что надо делать именно так, и передаст в макрос условное математическое выражение, то к дебаггеру может внезапно прийти доктор Ватсон или WER.
Решается быстро: параметр просто оборачивается в одну пару круглых скобок.
#define TITANGETDRX(titantype) UE_DR0 + (((titantype) >> 8) & 0xF)
Аналогичные срабатывания:
V560 A part of conditional expression is always true: * memorySize <= 512. The value range of unsigned char type: [0, 255]. TraceRecord.cpp 239
//See https://www.felixcloutier.com/x86/FXSAVE.html, max 512 bytes
#define memoryContentSize 512
static void HandleZydisOperand(
const Zydis & zydis, int opindex,
DISASM_ARGTYPE* argType, duint* value,
unsigned char memoryContent[memoryContentSize],
unsigned char* memorySize)
{
....
case ZYDIS_OPERAND_TYPE_MEMORY:
{
*argType = arg_memory;
const auto & mem = op.mem;
if(mem.segment == ArchValue(ZYDIS_REGISTER_FS, ZYDIS_REGISTER_GS))
{
*value += ThreadGetLocalBase(ThreadGetId(hActiveThread));
}
*memorySize = op.size / 8;
if(*memorySize <= memoryContentSize && DbgMemIsValidReadPtr(*value)) // <=
{
MemRead(*value, memoryContent, max(op.size / 8, sizeof(duint)));
}
}
break;
....
}
Снова код, в котором разработчик отладчика оставил комментарий в виде ссылки на документацию. В этот раз — на инструкцию FXSAVE архитектуры x86. Эта инструкция сохраняет состояние математического сопроцессора, а также регистров MMX, XMM и MXCSR в область памяти размером 512 байт. Коллега поставил под сомнение документацию из Интернета, посчитав её недостоверной, из-за чего завязался громкий спор на несколько минут. В процессе я вспомнил, что где-то уже видел эту таблицу...
С хитрой ухмылкой на лице я подхожу к книжной полке и медленно достаю из неё второй том "Руководства разработчика программного обеспечения для архитектуры IA-32", выпущенного Intel аж в 2002 году! Книга описывает каждую поддерживаемую инструкцию тогда ещё свеженького и горячего процессора Pentium 4. Тяжёлая и толстая книга с громким хлопком приземляется на мой стол, и я открываю её на странице, где заранее оставил закладку. Перед моим оппонентом предстаёт во всём своём величии та же самая таблица, что и в электронном виде. На этом спор закончился. Я же лишь могу предложить убрать избыточную проверку размера. unsigned char не может принимать значение больше 255 — он ростом невелик, чтобы дотянуться до всех данных инструкции FXSAVE.
....
*memorySize = op.size / 8;
if(DbgMemIsValidReadPtr(*value))
{
MemRead(*value, memoryContent, max(op.size / 8, sizeof(duint)));
}
....
V1048 The 'titsize' variable was assigned the same value. cmd-breakpoint-control.cpp 427
bool cbDebugSetHardwareBreakpoint(int argc, char* argv[])
{
....
DWORD titsize = UE_HARDWARE_SIZE_1;
if(argc > 3)
{
duint size;
if(!valfromstring(argv[3], &size))
return false;
switch(size)
{
case 1:
titsize = UE_HARDWARE_SIZE_1; // <=
break;
case 2:
titsize = UE_HARDWARE_SIZE_2;
break;
case 4:
titsize = UE_HARDWARE_SIZE_4;
break;
#ifdef _WIN64
case 8:
titsize = UE_HARDWARE_SIZE_8;
break;
#endif // _WIN64
default:
titsize = UE_HARDWARE_SIZE_1; // <=
dputs(QT_TRANSLATE_NOOP("DBG", "Invalid size, using 1"));
break;
}
....
}
....
}
Обычно числовые переменные инициализируют с нулевым значением. Никто не запрещает делать это конкретным значением. Кроме того, никто не запрещает перезаписывать переменную тем же значением в ходе выполнения программы, но зачем... Для убедительности или подстраховки?
V1037 Two or more case-branches perform the same actions. Check lines: 42, 45 commandparser.cpp 42
Command::Command(const String & command)
{
ParseState state = Default;
int len = (int)command.length();
for(int i = 0; i < len; i++)
{
char ch = command[i];
switch(state)
{
....
case Escaped:
switch(ch)
{
case '\t':
case ' ':
dataAppend(' ');
break;
case ',':
dataAppend(ch); // <=
break;
case '\"':
dataAppend(ch); // <=
break;
default:
dataAppend('\\');
dataAppend(ch);
break;
}
state = Default;
break;
....
}
}
}
У меня сложилось впечатление, что x64dbg писался либо по принципу "кабы что не вышло", либо "и так сойдёт". Если отталкиваться от первой версии, то вполне логично, что весь проект набит одинаковыми операциями и повторными присвоениями. Если от второй, то для инструмента с использованием в среде высокой ответственности это непозволительная рассеянность.
В этом блоке кода есть одновременно fallthrough и дублирующиеся части. Для табуляции и пробела применён fallthrough, а значит автор явно уверен, что оба символа без проблем допишут к данным именно пробел. Что же не так с запятой или двойной кавычкой, почему нельзя было сделать fallthrough и для этой пары символов? Совершается одно и то же действие: дописывается значение переменной ch, а не какой-либо другой символ. Если здесь применить fallthrough, как для пробела и табуляции, катастрофы не случится:
switch(ch)
{
case '\t':
case ' ':
dataAppend(' ');
break;
case ',':
case '\"':
dataAppend(ch); // <=
break;
default:
dataAppend('\\');
dataAppend(ch);
break;
}
Выглядит как экономия на спичках байтах, но на самом деле на глазах читающего код и процессорном времени его мозга для осознания происходящего.
V1109 The 'InitCommonControls' function is deprecated. Consider switching to an equivalent newer function. x64dbg_launcher.cpp 426
int APIENTRY WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance,
LPSTR lpCmdLine, int nShowCmd)
{
InitCommonControls();
....
}
Эта функция объявлена устаревшей не просто так. Во-первых, она засоряет таблицу импортов. Во-вторых, существуют манифесты приложений, поддержку которых ввели как раз в Windows XP! Сначала с их помощью можно было включать поддержку тем оформления Windows в окнах приложения и выбирать определённые версии библиотек, а с выходом Windows Vista — устанавливать уровень привилегий приложения. На секундочку, для сборки используется Visual Studio 2013, где их даже можно встраивать! Это не настолько новая возможность, чтобы ей пренебрегать — функционал интеграции манифеста прямо в проекте появился в Visual Studio 2005. Пришлось сдуть пыль с этой IDE, чтобы проверить, не подвела ли меня память.
Включение манифеста в приложения или библиотеки Windows уже давно является стандартным действием при разработке, которое выполняется один раз, и больше к нему не возвращаются. Без манифеста, к слову, некоторые функции будут работать неправильно.
V1109 The 'PathRemoveFileSpecW' function is deprecated. Consider switching to an equivalent newer function. x64dbg_launcher.cpp 114
static HRESULT AddDesktopShortcut(TCHAR* szPathOfFile,
const TCHAR* szNameOfLink)
{
HRESULT hRes = NULL;
//Get the working directory
TCHAR pathFile[MAX_PATH + 1];
_tcscpy_s(pathFile, szPathOfFile);
PathRemoveFileSpec(pathFile);
....
}
Есть ещё несколько устаревших вызовов, например, к PathRemoveFileSpecW через макрос. Это функция для удаления замыкающего обратного слеша из пути. Вместо неё рекомендовано использование PathCchRemoveFileSpec, но эта функция доступна только с Windows 8. Поскольку приложение осознанно пишется с поддержкой Windows XP, такое срабатывание можно подавить, если необходимо работать с устаревшими функциями:
PathRemoveFileSpec(pathFile); //-V1109 //-VH"2078475722"
Заметьте, что мы использовали новый механизм — подавление с хэшем. Оно позволяет автоматически снимать все метки False Alarm в случае, если в строке изменился код. Благодаря этому можно обеспечить защиту от замены одной устаревшей функции на другую. То есть ручное разворачивание макроса до одной из версий функции (PathRemoveFileSpecW для Unicode, PathRemoveFileSpecA для ANSI) или замена другой функцией нарушит соответствие хэша строке, и анализатор вновь сообщит о проблеме с диагностикой V1109 в этой строке, если она возникнет.
Аналогичные срабатывания:
Существует мнение, что для починки одного паяльника необходим второй паяльник. К отладчику, технически, применимо то же самое утверждение. Как отлаживать отладчик? Возможно, стоит попробовать поискать в нём баги с помощью статического анализатора. Например, с помощью PVS-Studio? x64dbg тоже следит за вами, записывает потраченное на него время, поэтому цените этот ограниченный ресурс!
На этом история не заканчивается: ещё предстоит сделать "шаг с выходом" в GUI. Какие неожиданности нам приготовила обвязка из Qt? Об этом во второй части. Оставайтесь с нами, не переключайте контекст процессора!
0