To get a trial key
fill out the form below
Team License (standard version)
Enterprise License (extended version)
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Request our prices
New License
License Renewal
--Select currency--
USD
EUR
GBP
RUB
* By clicking this button you agree to our Privacy Policy statement

** This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
Message submitted.

Your message has been sent. We will email you at


If you haven't received our response, please do the following:
check your Spam/Junk folder and click the "Not Spam" button for our message.
This way, you won't miss messages from our team in the future.

>
>
>
Nice Chromium and clumsy memset

Nice Chromium and clumsy memset

Jan. 26, 2018
Author:

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

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.

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

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

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

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.

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

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++]; });
}

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

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

Option 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;
}

Option N2.

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.

Popular related articles
How PVS-Studio Proved to Be More Attentive Than Three and a Half Programmers

Date: 10.22.2018

Author: Andrey Karpov

Just like other static analyzers, PVS-Studio often produces false positives. What you are about to read is a short story where I'll tell you how PVS-Studio proved, just one more time, to be more atte…
The Last Line Effect

Date: 05.31.2014

Author: Andrey Karpov

I have studied many errors caused by the use of the Copy-Paste method, and can assure you that programmers most often tend to make mistakes in the last fragment of a homogeneous code block. I have ne…
The Ultimate Question of Programming, Refactoring, and Everything

Date: 04.14.2016

Author: Andrey Karpov

Yes, you've guessed correctly - the answer is "42". In this article you will find 42 recommendations about coding in C++ that can help a programmer avoid a lot of errors, save time and effort. The au…
PVS-Studio for Java

Date: 01.17.2019

Author: Andrey Karpov

In the seventh version of the PVS-Studio static analyzer, we added support of the Java language. It's time for a brief story of how we've started making support of the Java language, how far we've co…
Appreciate Static Code Analysis!

Date: 10.16.2017

Author: Andrey Karpov

I am really astonished by the capabilities of static code analysis even though I am one of the developers of PVS-Studio analyzer myself. The tool surprised me the other day as it turned out to be sma…
Characteristics of PVS-Studio Analyzer by the Example of EFL Core Libraries, 10-15% of False Positives

Date: 07.31.2017

Author: Andrey Karpov

After I wrote quite a big article about the analysis of the Tizen OS code, I received a large number of questions concerning the percentage of false positives and the density of errors (how many erro…
The way static analyzers fight against false positives, and why they do it

Date: 03.20.2017

Author: Andrey Karpov

In my previous article I wrote that I don't like the approach of evaluating the efficiency of static analyzers with the help of synthetic tests. In that article, I give the example of a code fragment…
Free PVS-Studio for those who develops open source projects

Date: 12.22.2018

Author: Andrey Karpov

On the New 2019 year's eve, a PVS-Studio team decided to make a nice gift for all contributors of open-source projects hosted on GitHub, GitLab or Bitbucket. They are given free usage of PVS-Studio s…
The Evil within the Comparison Functions

Date: 05.19.2017

Author: Andrey Karpov

Perhaps, readers remember my article titled "Last line effect". It describes a pattern I've once noticed: in most cases programmers make an error in the last line of similar text blocks. Now I want t…
Static analysis as part of the development process in Unreal Engine

Date: 06.27.2017

Author: Andrey Karpov

Unreal Engine continues to develop as new code is added and previously written code is changed. What is the inevitable consequence of ongoing development in a project? The emergence of new bugs in th…

Comments (0)

Next comments

This site is protected by reCAPTCHA and the Google Privacy Policy and Terms of Service apply.
This website uses cookies and other technology to provide you a more personalized experience. By continuing the view of our web-pages you accept the terms of using these files. If you don't want your personal data to be processed, please, leave this site.
Learn More →
Accept