Предлагаем вашему вниманию цикл статей, посвященных рекомендациям по написанию качественного кода на примере ошибок, найденных в проекте Chromium. Это первая часть, которая будет посвящена функции memset.
Господа программисты, с функцией memset надо что-то делать в C++ программах! Вернее, даже сразу понятно что делать - её надо прекратить использовать. В своё время я написал статью "Самая опасная функция в мире С/С++". Я думаю, несложно догадаться, что речь в статье идёт как раз о memset.
Однако, не буду голословным и ещё раз на примерах продемонстрирую опасность этой функции. Код проекта Chromium и используемых в нём библиотек очень качественный. Разработчики Google уделяют много внимания тестам и использованию различного инструментария для выявления дефектов. Например, Google разработал такие инструменты, как AddressSanitizer, ThreadSanitizer и MemorySanitizer.
В результате, ошибок, связанных с функций memset, очень мало, но печально, что они всё-таки есть. Очень качественный проект, но всё равно они есть!
Давайте посмотрим, что я заметил в процессе разбора отчета, выданного PVS-Studio. Как я писал в вводной статье, отчёт я смотрел достаточно бегло, поэтому могут быть и другие, незамеченные мной ошибки. Тем не менее, найденных дефектов нам будет достаточно для обсуждения функции malloc.
Первый тип ошибок связан с неправильным вычислением размера буфера. Или, другими словами, проблема в том, что возникает путаница между размером массива в байтах и количеством элементов в массиве. Подобные ошибки можно классифицировать как CWE-682: Incorrect Calculation.
Первый пример ошибки взят непосредственно из кода проекта Chromium. Обратите внимание, что массивы text и unmodified_text состоят из юникодных символов.
#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];
....
};В результате, заполняется нулями только половина элементов в этих массивах:
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:
Второй пример ошибки взят из используемой в Chromium библиотеки WebRTC. Ошибка аналогична предыдущей: не учтено, что элементы массива имеют тип int64_t.
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).
Да, отказаться от memset в коде на языке C невозможно. Однако, если речь идёт о C++, то давайте забудем про эту функцию. Не используйте в C++ коде функцию memset. Не используйте и точка.
На что заменить вызов memset?
Во-первых, можно использовать функцию std::fill. В этом случае заполнение массива будет выглядеть так:
fill(begin(key_event->text), end(key_event->text), 0);Во-вторых, часто вообще не надо использовать вызов специальных функций. Как правило, функция memset служит для инициализации локальных массивов и структур. Классика:
HDHITTESTINFO hhti;
memset(&hhti, 0, sizeof(hhti));Но ведь можно написать гораздо проще и надёжнее:
HDHITTESTINFO hhti = {};
Если речь идёт о конструкторе:
class C
{
int A[100];
public:
C() { memset(A, 0, sizeof(A)); }
};То можно написать:
class C
{
int A[100] = {};
public:
C() { }
};Иногда забывают, что второй аргумент задаёт значение одного единственного байта, который используется для заполнения буфера. С толку сбивает то, что второй аргумент функции memset имеет тип int. В результате возникают ошибки, которые можно классифицировать как CWE-628: Function Call with Incorrectly Specified Arguments.
Рассмотрим пример подобной ошибки, которую я заметил в движке V8, используемом в проекте Chromium.
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:
Программист решил заполнить блоки памяти значением 0x0BADC0DE, чтобы при отладке было легче понять что к чему. Однако, области памяти будут заполнены байтом со значением 0xDE.
То, что делает программист в коде, является низкоуровневой операцией и здесь без memset обойтись сложнее, чем в ситуациях, описанных ранее. Размер буферов не кратен 4 байтам, поэтому не получится как раньше использовать std::fill. Придётся написать и использовать собственную функцию.
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<char *>(buf), size,
[&] { if (n == 4) n = 0; return badcode[n++]; });
}Рекомендация
Какой-то специальной рекомендации здесь нет. Зато мы вновь убедились, что функция memset на самом деле здесь не нужна, так как не решает поставленную перед программистом задачу.
Функцию memset используют для затирания приватных данных после того, как они перестают быть нужны. Это неправильно. Если буфер с приватными данными после вызова функции memset никак не используется, то компилятор вправе удалить вызов этой функции. Этот дефект классифицируется как CWE-14: Compiler Removal of Code to Clear Buffers.
Я уже предвижу возражения, что вызов memset компилятору нельзя убирать. Можно. И он это делает с целью оптимизации. Чтобы разобраться в теме, предлагаю внимательно изучить следующую статью "Безопасная очистка приватных данных".
Давайте посмотрим, как выглядят такие ошибки на практике. Начнём мы с библиотеки WebRTC, используемой в Chromium.
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 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 версии.
Аяяй! Пароль останется болтаться где-то в памяти и, теоретически, может быть куда-то отправлен. Я серьезно, такое действительно бывает.
В этой же библиотеке я встретил ещё 3 подобных ошибки. Описывать я их не буду, так как они однотипны. Приведу только соответствующие сообщения анализатора:
Рекомендация
Никогда не используйте функцию memset для затирания приватных данных!
Следует использовать специализированные функции очистки памяти, которые не могут быть удалены компилятором в процессе оптимизации кода.
Примечание. Это касается не только С++ программистов, но C программистов тоже.
В Visual Studio, например, можно использовать RtlSecureZeroMemory. Начиная с C11 существует функция memset_s. В случае необходимости вы можете создать свою собственную безопасную функцию. В интернете достаточно много примеров, как её сделать. Вот некоторые из вариантов.
Вариант N1.
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.
void secure_zero(void *s, size_t n)
{
volatile char *p = s;
while (n--) *p++ = 0;
}В случае проекта Chromium, наверное, рационально использовать функцию OPENSSL_cleanse.
Если Вы пишите программу на C++ и захотели написать вызов функции memset, то остановитесь. Скорее всего, вы отлично обойдётесь без этой опасной функции.