# Nice Chromium and clumsy memset We would like to suggest reading the series of articles dedicated to the recommendations on writing code of high quality using the examples of errors found in the Chromium project\. This is the first part which will be devoted to the memset function\. ![0553_Chromium_1_memset/image1.png](https://import.viva64.com/docx/blog/0553_Chromium_1_memset/image1.png) We must do something about the_ memset_ function in C\+\+ programs\! Rather, it is clear what we must do at once \- we have to stop using it\. I wrote the article "[The most dangerous function in the C/C\+\+ world](https://pvs-studio.com/en/blog/posts/cpp/0360/)" at the time\. I think it is easy to guess that this article will be exactly about _memset_\. However, I will not waste words, so I am going to demonstrate the danger of this function once again with the examples\. The code of the Chromium project and the libraries used in it are of very high quality\. Google developers pay much attention to the tests and the use of various tools for detecting defects\. For instance, Google has developed such [tools](https://github.com/google/sanitizers) as AddressSanitizer, ThreadSanitizer and MemorySanitizer\. As a result, there are few errors related to _memset_ function, but sadly, that they are still presented\. Despite the errors, it is a very qualitative project\! Let's see what I noticed during studying the report issued by the PVS\-Studio\. As I wrote in the [introductory article](https://pvs-studio.com/en/blog/posts/cpp/0552/), I looked through the report quite fluently, so there may be other, unnoticed errors\. However, the found defects will be enough for us to discuss the _malloc_ function\. ## Incorrectly Calculated Buffer Size The first type of errors is related to the incorrect calculation of the buffer size\. Or, in other words, the problem is that there is confusion between the size of the array in bytes, and the number of elements in the array\. Such errors may be classified as [CWE\-682](https://cwe.mitre.org/data/definitions/682.html): Incorrect Calculation\. The first example of the error is taken directly from the Chromium project code\. Note that the arrays _text_ and _unmodified\_text_ consist of unicode characters\. ```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]; .... }; ``` As a result, only half of the elements in these arrays is filled with zeros: ```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 warnings: * [V512](https://pvs-studio.com/en/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 The second example of the error is taken from the WebRTC library used in Chromium\. The error is similar to the previous bug: it is not taken into account that the elements in the array are of _int64\_t_ type\. ```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); } ``` Here only the first element of the array is set to null, and one byte in the second element\. PVS\-Studio warning: V512 CWE\-682 A call of the 'memset' function will lead to underflow of the buffer '\_jumpBuf'\. rtt\_filter\.cc 52 **Recommendation** To avoid such errors do not use _memset_ any more\. You may be really careful, but sooner or later errors will get passed around in your project anyway\. In Chromium the situation is quite favorable\. Nevertheless, in other projects it is a very common problem \([proof](https://pvs-studio.com/en/blog/examples/v512/)\)\. Yes, it is impossible to avoid the use of _memset_ in C code\. However, if we are talking about C\+\+, let's forget about this function\. Do not use _memset_ function in C\+\+ code\. Do not use, end of story\. How to replace the _memset_ call? Firstly, you can use the [std: fill](https://en.cppreference.com/w/cpp/algorithm/fill) function\. In this case, a filling of an array will look like this: ```cpp fill(begin(key_event->text), end(key_event->text), 0); ``` Secondly, you should not often use a call of special functions\. Typically, _memset_ function serves to initialize local arrays and structures\. Classic example: ```cpp HDHITTESTINFO hhti; memset(&hhti, 0, sizeof(hhti)); ``` But you can write much easier and safer: ```cpp HDHITTESTINFO hhti = {}; ``` If we are talking about the constructor: ```cpp class C { int A[100]; public: C() { memset(A, 0, sizeof(A)); } }; ``` It is possible to write as follows: ```cpp class C { int A[100] = {}; public: C() { } }; ``` ## Incorrect Expectations from Memset Developers sometimes forget that the second argument sets the value of a single byte that is used to fill the buffer\. What is confusing is that the second argument of the_ memset_ function is of _int_ type\. As a result, such errors appear, which can be classified as [CWE\-628](https://cwe.mitre.org/data/definitions/628.html): Function Call with Incorrectly Specified Arguments\. Let's look at the example of such an error that I noticed in the V8 engine, used in the Chromium project\. ```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 warnings: * [V575](https://pvs-studio.com/en/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 A developer decided to fill the memory blocks with the _0x0BADC0DE_ value, so that it was easier to understand the situation when debugging\. However, the memory space will be filled with the byte with the _0xDE_ value\. What a programmer does in code is a low\-level operation and here it is harder to do without _memset _than in the situations described earlier\. The buffers' size is not multiple to 4 bytes, so a usage of _std::fill_ will not work as earlier\. A programmer will have to write and use his own function\. ```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++]; }); } ``` **Recommendation** There is no any special recommendation\. Once again, we have seen that _memset_ function is actually not needed here, as it does not solve the programmer task\. ## Error of Private Data Clearing _Memset_ function is_ _used for clearing of private data after it is no longer needed\. This is wrong\. If a buffer with private data is not used in any way after the call of _memset_, the compiler may remove the call to this function\. This defect is classified as [CWE\-14](https://cwe.mitre.org/data/definitions/14.html): Compiler Removal of Code to Clear Buffers\. I already anticipate the objection that a compiler cannot remove a _memset_ calling\. It can\. It does it in terms of optimization\. To understand the topic, I would like to suggest to carefully study the following article "[Safe Clearing of Private Data](https://pvs-studio.com/en/blog/posts/cpp/0388/)"\. Let's see how these errors look like in practice\. We will start the WebRTC library used in 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 warning: [V597](https://pvs-studio.com/en/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_ function will be removed by a compiler in a Release version with a probability close to 100%\. Ow ow ow\! The password will remain hanging out somewhere in memory and, theoretically, can be sent somewhere\. I am serious, this really [happens](https://pvs-studio.com/en/blog/posts/cpp/k0041/)\. In the same library I came across 3 more similar errors\. I will not describe them because they are similar\. I will just cite only the appropriate analyzer messages: * 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 **Recommendation** Never use the _memset_ function for clearing private data\! You should use special memory clearing functions that the compiler is not allowed to remove for its optimization purposes\. Note\. This concerns not only C\+\+ programmers, but also C programmers as well\. Visual Studio, for instance, offers the [_RtlSecureZeroMemory_](https://msdn.microsoft.com/en-us/library/windows/hardware/ff562768%28v=vs.85%29.aspx) function\. Starting with C11, you can use the [_memset\_s_](https://en.cppreference.com/w/c/string/byte/memset) function\. If necessary, you can create your own secure function\. There are a lot of examples in the internet, how to write it\. Here are some of the options\. Option [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; } ``` Option [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; } ``` In the case of Chromium, probably, it is reasonable to use the function _OPENSSL\_cleanse_\. ## Conclusion If you are writing a C\+\+ program and you want to write a function call _to memset_, then stop\. Most likely, you will do great without this dangerous function\.