17. Use dedicated functions to clear private data

The fragment is taken from the Apache HTTP Server project. The error is detected by the following PVS-Studio diagnostic: V597 The compiler could delete the 'memset' function call, which is used to flush 'x' buffer. The RtlSecureZeroMemory() function should be used to erase the private data.

static void MD4Transform(
  apr_uint32_t state[4], const unsigned char block[64])
{
  apr_uint32_t a = state[0], b = state[1],
               c = state[2], d = state[3],
               x[APR_MD4_DIGESTSIZE];  
  ....
  /* Zeroize sensitive information. */
  memset(x, 0, sizeof(x));
}

Explanation

In this code the programmer uses a call of the memset() function to erase private data. But it's not the best way to do that because the data won't actually be erased. To be more exact, whether or not they will be erased depends on the compiler, its settings, and the Moon phase.

Try to look at this code from the compiler's viewpoint. It does its best to make your code work as fast as possible, so it carries out a number of optimizations. One of them is to remove the calls of functions which don't affect the program's behavior, and are therefore excessive from the viewpoint of the C/C++ language. This is exactly the case with the memset() function in the code sample above. True, this function changes the 'x' buffer, but this buffer is not used anywhere after that, which means the call of the memset() function can - and ought to - be deleted.

Important! What I'm telling you now is not a theoretical model of the compiler's behavior - it's a real-life one. In such cases, the compiler does remove the calls of the memset() function. You can do a few experiments to check it for yourself. For more details and examples on this issue, please see the following articles:

1. Security, security! But do you test it?
2. Safe Clearing of Private Data.
3. V597. The compiler could delete the 'memset' function call, which is used to flush 'Foo' buffer. The RtlSecureZeroMemory() function should be used to erase the private data
4. Zero and forget -- caveats of zeroing memory in C (see also the discussion of this article).
5. MSC06-C. Beware of compiler optimizations.

What makes this error with removed memset() calls especially tricky, is its being very hard to track. When working in the debugger, you will most likely be dealing with un-optimized code, with the function call still there. You can only find the error when studying the assembler listing, generated when building the optimized application version.

Some programmers believe that it has to do with a bug in the compiler, and that it has no right to throw away the calls of such an important function as memset(). But this is not the case. This function is by no means more, or less, important than any other, so the compiler has full right to optimize the code where it is called. After all, such code may turn out to be excessive indeed.

Correct code

memset_s(x, sizeof(x), 0, sizeof(x));

or

RtlSecureZeroMemory(x, sizeof(x));

Recommendation

You should use special memory clearing functions that the compiler is not allowed to remove for its optimization purposes.

Visual Studio, for instance, offers the RtlSecureZeroMemory function; and starting with C11, you can use the memset_s function. If necessary, you can even create a safe function of your own - there are lots of examples on the Internet. Here is a couple of them.

Version No.1.

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

Version No.2.

void secure_zero(void *s, size_t n)
{
    volatile char *p = s;
    while (n--) *p++ = 0;
}

Some programmers even go further, and implement functions to fill the array with pseudo-random values, these functions running at different times to ensure better protection from time-measuring attacks. You can find the implementations of such functions on the internet, too.