# Красивый Chromium и корявый memset Предлагаем вашему вниманию цикл статей, посвященных рекомендациям по написанию качественного кода на примере ошибок, найденных в проекте Chromium\. Это первая часть, которая будет посвящена функции memset\. ![0553_Chromium_1_memset_ru/image1.png](https://import.viva64.com/docx/blog/0553_Chromium_1_memset_ru/image1.png) Господа программисты, с функцией _memset_ надо что\-то делать в C\+\+ программах\! Вернее, даже сразу понятно что делать \- её надо прекратить использовать\. В своё время я написал статью "[Самая опасная функция в мире С/С\+\+](https://pvs-studio.ru/ru/blog/posts/cpp/0360/)"\. Я думаю, несложно догадаться, что речь в статье идёт как раз о _memset_\. Однако, не буду голословным и ещё раз на примерах продемонстрирую опасность этой функции\. Код проекта Chromium и используемых в нём библиотек очень качественный\. Разработчики Google уделяют много внимания тестам и использованию различного инструментария для выявления дефектов\. Например, Google разработал такие [инструменты](https://github.com/google/sanitizers), как AddressSanitizer, ThreadSanitizer и MemorySanitizer\. В результате, ошибок, связанных с функций _memset_, очень мало, но печально, что они всё\-таки есть\. Очень качественный проект, но всё равно они есть\! Давайте посмотрим, что я заметил в процессе разбора отчета, выданного PVS\-Studio\. Как я писал в [вводной статье](https://pvs-studio.ru/ru/blog/posts/cpp/0552/), отчёт я смотрел достаточно бегло, поэтому могут быть и другие, незамеченные мной ошибки\. Тем не менее, найденных дефектов нам будет достаточно для обсуждения функции _malloc_\. ## Неправильно вычисленный размер буфера Первый тип ошибок связан с неправильным вычислением размера буфера\. Или, другими словами, проблема в том, что возникает путаница между размером массива в байтах и количеством элементов в массиве\. Подобные ошибки можно классифицировать как [CWE\-682](https://cwe.mitre.org/data/definitions/682.html): Incorrect Calculation\. Первый пример ошибки взят непосредственно из кода проекта Chromium\. Обратите внимание, что массивы _text_ и _unmodified\_text_ состоят из юникодных символов\. ```cpp #if defined(WIN32) typedef wchar_t WebUChar; #else typedef unsigned short WebUChar; #endif static const size_t kTextLengthCap = 4; class WebKeyboardEvent : public WebInputEvent { .... WebUChar text[kTextLengthCap]; WebUChar unmodified_text[kTextLengthCap]; .... }; ``` В результате, заполняется нулями только половина элементов в этих массивах: ```cpp WebKeyboardEvent* BuildCharEvent(const InputEventData& event) { WebKeyboardEvent* key_event = new WebKeyboardEvent(....); .... memset(key_event->text, 0, text_length_cap); memset(key_event->unmodified_text, 0, text_length_cap); .... } ``` Предупреждения PVS\-Studio: * [V512](https://pvs-studio.ru/ru/docs/warnings/v512/) CWE\-682 A call of the 'memset' function will lead to underflow of the buffer 'key\_event\-\>text'\. event\_conversion\.cc 435 * V512 CWE\-682 A call of the 'memset' function will lead to underflow of the buffer 'key\_event\-\>unmodified\_text'\. event\_conversion\.cc 436 Второй пример ошибки взят из используемой в Chromium библиотеки WebRTC\. Ошибка аналогична предыдущей: не учтено, что элементы массива имеют тип _int64\_t_\. ```cpp class VCMRttFilter { .... enum { kMaxDriftJumpCount = 5 }; .... int64_t _jumpBuf[kMaxDriftJumpCount]; int64_t _driftBuf[kMaxDriftJumpCount]; .... }; void VCMRttFilter::Reset() { _gotNonZeroUpdate = false; _avgRtt = 0; _varRtt = 0; _maxRtt = 0; _filtFactCount = 1; _jumpCount = 0; _driftCount = 0; memset(_jumpBuf, 0, kMaxDriftJumpCount); memset(_driftBuf, 0, kMaxDriftJumpCount); } ``` Здесь вообще обнуляется только первый элемент массива и один байт во втором элементе\. Предупреждение PVS\-Studio: V512 CWE\-682 A call of the 'memset' function will lead to underflow of the buffer '\_jumpBuf'\. rtt\_filter\.cc 52 **Рекомендация** Способ избежать таких ошибок \- не использовать более _memset_\. Можно быть очень аккуратным, но рано или поздно ошибки всё равно просочатся в ваш проект\. Это в проекте Chromium картина хорошая\. В других проектах \- это очень распространенная проблема \([proof](https://pvs-studio.ru/ru/blog/examples/v512/)\)\. Да, отказаться от _memset_ в коде на языке C невозможно\. Однако, если речь идёт о C\+\+, то давайте забудем про эту функцию\. Не используйте в C\+\+ коде функцию _memset_\. Не используйте и точка\. На что заменить вызов _memset_? Во\-первых, можно использовать функцию [std::fill](https://en.cppreference.com/w/cpp/algorithm/fill)\. В этом случае заполнение массива будет выглядеть так: ```cpp fill(begin(key_event->text), end(key_event->text), 0); ``` Во\-вторых, часто вообще не надо использовать вызов специальных функций\. Как правило, функция _memset_ служит для инициализации локальных массивов и структур\. Классика: ```cpp HDHITTESTINFO hhti; memset(&hhti, 0, sizeof(hhti)); ``` Но ведь можно написать гораздо проще и надёжнее: ```cpp HDHITTESTINFO hhti = {}; Если речь идёт о конструкторе: class C { int A[100]; public: C() { memset(A, 0, sizeof(A)); } }; ``` То можно написать: ```cpp class C { int A[100] = {}; public: C() { } }; ``` ## Неправильные ожидания от memset Иногда забывают, что второй аргумент задаёт значение одного единственного байта, который используется для заполнения буфера\. С толку сбивает то, что второй аргумент функции _memset_ имеет тип _int_\. В результате возникают ошибки, которые можно классифицировать как [CWE\-628](https://cwe.mitre.org/data/definitions/628.html): Function Call with Incorrectly Specified Arguments\. Рассмотрим пример подобной ошибки, которую я заметил в движке V8, используемом в проекте Chromium\. ```cpp void i::V8::FatalProcessOutOfMemory( const char* location, bool is_heap_oom) { .... char last_few_messages[Heap::kTraceRingBufferSize + 1]; char js_stacktrace[Heap::kStacktraceBufferSize + 1]; i::HeapStats heap_stats; .... memset(last_few_messages, 0x0BADC0DE, Heap::kTraceRingBufferSize + 1); memset(js_stacktrace, 0x0BADC0DE, Heap::kStacktraceBufferSize + 1); memset(&heap_stats, 0xBADC0DE, sizeof(heap_stats)); .... } ``` Предупреждения PVS\-Studio: * [V575](https://pvs-studio.ru/ru/docs/warnings/v575/) CWE\-628 The 'memset' function processes value '195936478'\. Inspect the second argument\. api\.cc 327 * V575 CWE\-628 The 'memset' function processes value '195936478'\. Inspect the second argument\. api\.cc 328 * V575 CWE\-628 The 'memset' function processes value '195936478'\. Inspect the second argument\. api\.cc 329 Программист решил заполнить блоки памяти значением _0x0BADC0DE_, чтобы при отладке было легче понять что к чему\. Однако, области памяти будут заполнены байтом со значением _0xDE_\. То, что делает программист в коде, является низкоуровневой операцией и здесь без _memset_ обойтись сложнее, чем в ситуациях, описанных ранее\. Размер буферов не кратен 4 байтам, поэтому не получится как раньше использовать _std::fill_\. Придётся написать и использовать собственную функцию\. ```cpp void Fill_0x0BADC0DE(void *buf, const size_t size) { const unsigned char badcode[4] = { 0xDE, 0xC0, 0xAD, 0x0B }; size_t n = 0; generate_n(static_cast(buf), size, [&] { if (n == 4) n = 0; return badcode[n++]; }); } ``` **Рекомендация** Какой\-то специальной рекомендации здесь нет\. Зато мы вновь убедились, что функция _memset_ на самом деле здесь не нужна, так как не решает поставленную перед программистом задачу\. ## Ошибка затирания приватных данных Функцию _memset_ используют для затирания приватных данных после того, как они перестают быть нужны\. Это неправильно\. Если буфер с приватными данными после вызова функции _memset_ никак не используется, то компилятор вправе удалить вызов этой функции\. Этот дефект классифицируется как [CWE\-14](https://cwe.mitre.org/data/definitions/14.html): Compiler Removal of Code to Clear Buffers\. Я уже предвижу возражения, что вызов _memset_ компилятору нельзя убирать\. Можно\. И он это делает с целью оптимизации\. Чтобы разобраться в теме, предлагаю внимательно изучить следующую статью "[Безопасная очистка приватных данных](https://pvs-studio.ru/ru/blog/posts/cpp/0388/)"\. Давайте посмотрим, как выглядят такие ошибки на практике\. Начнём мы с библиотеки WebRTC, используемой в Chromium\. ```cpp void AsyncSocksProxySocket::SendAuth() { .... char * sensitive = new char[len]; pass_.CopyTo(sensitive, true); request.WriteString(sensitive); // Password memset(sensitive, 0, len); delete [] sensitive; DirectSend(request.Data(), request.Length()); state_ = SS_AUTH; } ``` Предупреждение PVS\-Studio: [V597](https://pvs-studio.ru/ru/docs/warnings/v597/) CWE\-14 The compiler could delete the 'memset' function call, which is used to flush 'sensitive' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. socketadapters\.cc 677 Функция _memset_ с вероятностью близкой к 100% будет удалена компилятором в Release версии\. Аяяй\! Пароль останется болтаться где\-то в памяти и, теоретически, может быть куда\-то отправлен\. Я серьезно, такое действительно [бывает](https://pvs-studio.ru/ru/blog/posts/cpp/k0041/)\. В этой же библиотеке я встретил ещё 3 подобных ошибки\. Описывать я их не буду, так как они однотипны\. Приведу только соответствующие сообщения анализатора: * V597 CWE\-14 The compiler could delete the 'memset' function call, which is used to flush 'sensitive' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. httpcommon\.cc 721 * V597 CWE\-14 The compiler could delete the 'memset' function call, which is used to flush 'sensitive' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. httpcommon\.cc 766 * V597 CWE\-14 The compiler could delete the 'memset' function call, which is used to flush 'sensitive' object\. The RtlSecureZeroMemory\(\) function should be used to erase the private data\. httpcommon\.cc 917 **Рекомендация** Никогда не используйте функцию _memset_ для затирания приватных данных\! Следует использовать специализированные функции очистки памяти, которые не могут быть удалены компилятором в процессе оптимизации кода\. Примечание\. Это касается не только С\+\+ программистов, но C программистов тоже\. В Visual Studio, например, можно использовать [_RtlSecureZeroMemory_](https://msdn.microsoft.com/en-us/library/windows/hardware/ff562768%28v=vs.85%29.aspx)\. Начиная с C11 существует функция [_memset\_s_](https://en.cppreference.com/w/c/string/byte/memset)\. В случае необходимости вы можете создать свою собственную безопасную функцию\. В интернете достаточно много примеров, как её сделать\. Вот некоторые из вариантов\. Вариант [N1](https://wiki.sei.cmu.edu/confluence/display/c/msc06-c.+beware+of+compiler+optimizations)\. ```cpp errno_t memset_s(void *v, rsize_t smax, int c, rsize_t n) { if (v == NULL) return EINVAL; if (smax > RSIZE_MAX) return EINVAL; if (n > smax) return EINVAL; volatile unsigned char *p = v; while (smax-- && n--) { *p++ = c; } return 0; } ``` Вариант [N2](https://stackoverflow.com/questions/13299420/mac-os-x-equivalent-of-securezeromemory-rtlsecurezeromemory/13299459)\. ```cpp void secure_zero(void *s, size_t n) { volatile char *p = s; while (n--) *p++ = 0; } ``` В случае проекта Chromium, наверное, рационально использовать функцию _OPENSSL\_cleanse_\. ## Заключение Если Вы пишите программу на C\+\+ и захотели написать вызов функции _memset_, то остановитесь\. Скорее всего, вы отлично обойдётесь без этой опасной функции\.