cc11

Missing the great reason for memset_s (C11)


I'm sure there are a lot of buffer overflow risks in coding, many of which are addressed by standard library's "_s" safe functions. Nonetheless, I find myself confused, from time to time, on some of them.

Let's say I have some fragment like this

uint8_t a[5];
...
size_t  z = 6;
...
memset(a, 0, z);  // Overflow!

Some compilers (C11) may advise better use of memset_s; as I'm a lousy programmer I just updated my code to this brand new thing, my way:

uint8_t  a[5];
...
rsize_t  max_array = 56;  // Slipped finger, head in the clouds, etc.
rsize_t  z = 6;
...
memset_s(a, max_array, 0, z);  // So what?

How would memset_s be any better than memset if I'm just adding the error to another parameter?

Where is the added safety, if adding a new parameter simply adds a new parameter that can be wrong. I could have corrected my code in the first revision, and still have a legal call to a well-defined operation on a buffer.

Left apart the case with an unchecked zero pointer, how could memset be that much worse than memset_s?

[EDIT]

With a little effort, I also found what setting was causing the warning in my setup. This may be of help for someone.

The warning comes from Clang-tidy, invoked in the Visual Studio extension "Clang power tools". In its default setup, it enables for Clang-tidy the checker "security.insecureAPI.DeprecatedOrUnsafeBufferHandling" which mimics the default MSVC warnings. Note that this isn't available from command line options (or at least is not listed among them), but read from a .clang-tidy file.
In the first runs, I didn't even have a .clang-tidy file, although that was enforced by default.

In my case, the reason for the message appearing in Clang-tidy and not with MSVC is straightforward.
I defined in my code

#define  _CRT_SECURE_NO_WARNINGS

to not be infested by MSVC warnings, but this method is ineffective with Clang-tidy, which kept nagging me (due to Clang power tools defaults)


Solution

  • I would not advocate the use of any of the controversial secure functions from Annex K because of portability issues: the original implementation on Microsoft platforms does not conform to the Standard specification and many other platforms deliberately chose not to implement them at all.

    Here are some features of memset_s that memset lacks:

    • null pointer check on the destination array.
    • partial sanity check on the block size: detecting sizes larger than RSIZE_MAX handles the case of erroneous sizes computed from data producing negative values. This alone should not have mandated a change of type from size_t to rsize_t, but does catch some programming errors, albeit at run time only.
    • memset_s() can not be elided: K.3.7.4.1: Unlike memset, any call to the memset_s function shall be evaluated strictly according to the rules of the abstract machine as described in (5.1.2.3). That is, any call to the memset_s function shall assume that the memory indicated by s and n may be accessible in the future and thus must contain the values indicated by c.

    I agree with you that the first two features offer limited protection as both cases would produce segmentation faults on protected memory architectures anyway and the error handling scheme can hardly do better than exit the program in such cases too.

    The third one is useful to ensure sensitive information contained in a local array is erased before the function returns preventing the compiler from optimizing out the code that clears the array. As memset_s support was kept optional, a new function memset_explicit was added with this exact purpose in C23 with the same prototype as memset. Similar functions were available under different names in some targets: explicit_memset (Oracle), or secure alternatives to clear an array: explicit_bzero (BSD), memzero_explicit (Linux) and SecureZeroMemory (Windows).

    Size errors such as the one you exhibit will go undetected and cause hard to find bugs. They typically occur when the target array is not defined locally and its size is passed separately from the pointer, otherwise sizeof array is a safer choice by any means.

    A typical error that bites even the most savvy programmers is this:

    void handle_array(some_type *dest, size_t size) {
        /* clear the array */
        memset(dest, 0, size);  // BUG: using number of elements instead of byte size
        for (size_t i = 0; i < size; i++) {
            /* do other stuff... */
        }
    }
    

    The compiler cannot detect this programming error and using memset_s would not catch it either.

    Another classic programming error is swapping the 0 and the size arguments.

    I personally use macros to avoid these pitfalls:

    #define array_clear(a, n)  memset(a, 0, sizeof(*(a)) * (n))
    

    Adding tagged pointers to the language, that would combine the type, address and length of the object and would be constructed automatically from array arguments or assignments would help avoid many such problems. Such departures from the current definition of the C language that are not present in C++ either are alas very unlikely to ever make it into a next version of the C Standard.