How strcat_s avoids buffer overflow problem?

Question

I have below piece of code

#include <stdio.h>
#include <string.h>

void fn(char *status, size_t maxLen)
{
    strcat(status, "1234567890");
}

int main()
{
    char status[5] = { 0 };
    size_t statusMaxLen = sizeof(status) / sizeof(status[0]);
    printf("%s%zu", "size of a status string ", statusMaxLen);
    fn(status, statusMaxLen);
    return 0;
}

I am getting

run-time check failure #2 - Stack around the variable was corrupted

erro when using strcat. If I replace strcat with strcat_s() like this

strcat_s(status, maxLen, "1234567890");

getting

Debug assertion failed. Buffer is too small

So only type of error is different but still my program crashes. The solution to avoid crash is to check the size before concatenating. In this case why I need strcat_s. strcat also will work fine when size check done. So what will be the real use of strcat_s here.

Whether I use strcat or strcat_s, I need to check the size before copying / concatenating. If I do size check, then why should I prefer strcat_s over strcat?

if((strlen("1234567890") + strlen(status)) < maxLen){ 
    strcat(status, "1234567890");}`

Answer 1

You should always check buffer sizes, regardless of which string concatenation function you use. That said, the code with strcat_s is better in two ways:

• This is no longer a buffer overflow vulnerability.

  A buffer overflow vulnerability occurs when an attacker is able to supply a piece of data that is too long, and that data then overwrites something outside of the buffer, in some cases allowing the attacker to take control of the process by tricking it to run malicious code or misbehave in some other way.

  Since strcat_s crashes instead of allowing this to happen, the behaviour is now predictable and cannot be exploited by an attacker.

• You can choose a more useful behaviour than crashing the process.

  You can set the constraint handler that gets invoked when strcat_s detects that the destination buffer is too small.

There isn't really any way around checking the buffer size. Even if you use strncat, you have the issue that strncat doesn't null-terminate the destination buffer if it is too small, which can cause all kinds of problems. _{Never mind, I mixed up strncpy and strncat. Note that the size argument to strncat probably doesn't do what you expect.}

Answer 2

Whether I use strcat or strcat_s, I need to check the size before copying / concatenating. If I do size check, then why should I prefer strcat_s over strcat?

You shouldn't. If you check the size in advance, strcat is faster and more portable than strcat_s.

The _s functions are dangerou_s in general, because they are poorly standardized and lack compiler support.

Buffer overruns are bad, but having your program crashing instead isn't an "advantage". Instead make sure that it doesn't crash at all.

The worst thing that could happen to your last example is that strlen goes on reading out of bounds when handed a too long or non-null terminated string, which would result in array-out-of-bounds access and potentially a seg fault crash.

As a rule of thumb, first sanitize your program's input, then apply the fastest possible function on the verified data.

Answer 3

strcat() operates on strings. So the buffer must be big enough to hold the terminating \0too. Or make sure that the string is shorter than the size of the buffer. Also, make sure that there is a \0 at the end of the string.

Also, if you read the documentation for strcat() you will understand that the buffer you provide is too small to hold the resulting string. strcat() will not allocate memory in order to accommodate longer strings.

---

I cannot test now, but you should probably have:

size_t statusMaxLen = sizeof(status) / sizeof(status[0]) - 1;

making statusMaxLen smaller with 1 (needed for \0).

Answer 4

Advantage of strcat_s() is that this error is guaranteed to be detected. With strcat(), if there is an overflow the behavior is undefined.

See the documentation:

the following errors are detected at runtime and call the currently installed constraint handler function:

• src or dest is a null pointer
• destsz is zero or greater than RSIZE_MAX
• there is no null terminator in the first destsz bytes of dest
• truncation would occur (the available space at the end of dest would not fit every character, including the null terminator, of src)
• overlap would occur between the source and the destination strings

Answer 5

The definition of strcat() clearly hints to why your program crashes:

It takes two arguments, i.e, two strings or character arrays, and stores the resultant concatenated string in the first string specified in the argument.

Your first argument status is not large enough to hold the concatenated string.

strcat will cause stack smashing, strcat_s will cause segfault. These are two different things as explained in:

Isnt Segmentation fault the same as the smashing the stack?

the difference between segment fault and Stack smashing detected