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.

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" 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 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, 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: 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.

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.

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).

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 function. In this case, a filling of an array will look like this:

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:

HDHITTESTINFO hhti;
memset(&hhti, 0, sizeof(hhti));

But you can write much easier and safer:

HDHITTESTINFO hhti = {};

If we are talking about the constructor:

class C
{
int A[100];
public:
C() { memset(A, 0, sizeof(A)); }
};

It is possible to write as follows:

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: 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.

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.

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 isused 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: 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".

Let's see how these errors look like in practice. We will start the WebRTC library used in Chromium.

PVS-Studio warning: 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.

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 function. Starting with C11, you can use the memset_s 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.

We use cookies for the analysis of events to improve our content and make user interaction more convenient. By continuing the view of our web-pages you accept the terms of using these files. You can find out more about cookie-files and privacy policy or close the notification, by clicking on the button.
Learn More →