7

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)

7
  • It's probably meant to be used like memset_s(a, sizeof a, 0, z); but I agree that it doesn't add much safety.
    – Ted Lyngmo
    Commented Jul 28, 2023 at 9:56
  • 3
    Only one compiler vendor pushes for those functions - MSVC, and it seems gnu libc doesn't support them at all. And you're not alone in thinking they're useless. This just another data point in support of MSVC being a weird compiler. Commented Jul 28, 2023 at 9:58
  • @HolyBlackCat, actually it came from Visual Studio, but not from MSVC: only activating additional analysis with Clang-tidy. The analysis with MSVC alone doesn't notify it.
    – LuC
    Commented Jul 28, 2023 at 13:42
  • @LuC This check doesn't seem to be enabled in clang-tidy itself by default. Perhaps something that VS enables. Commented Jul 28, 2023 at 17:20
  • 1
    @LuC: excellent edit! Let's all fight this infestation :)
    – chqrlie
    Commented Aug 4, 2023 at 17:46

1 Answer 1

7

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.

10
  • If you use visual studio, you don't need that macro because you get to see the function signature as you input the arguments.
    – machine_1
    Commented Jul 28, 2023 at 10:47
  • @chqrlie I totally agree with you, and my question is maybe more a request for comments. The complaint promoting memset_s came from Visual Studio, but only indirectly: activating the additional analysis with Clang-tidy, which eventually, in this setup suggested the change.
    – LuC
    Commented Jul 28, 2023 at 13:39
  • @machine_1: a good IDE does help avoid some problems, but the confusion between number of elements and byte size is not completely removed by this feature, especially when the variables are poorly named such as in my example: size is more ambiguous than count or element_count or even n.
    – chqrlie
    Commented Jul 28, 2023 at 13:57
  • 1
    The best thing about this Q&A is that not even one single person voted to close it as "opionion based" which is kind of amazing. :-) Got my vote too!
    – Ted Lyngmo
    Commented Jul 28, 2023 at 18:45
  • 1
    @LuC I understand and agree. This could have been closed as opinion-based if people had just been a little more trigger happy that day, but I'm glad it didn't.
    – Ted Lyngmo
    Commented Jul 31, 2023 at 11:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.