6

When I try to do things like this:

char* prefix = "Sector_Data\\sector";
char* s_num = "0";
std::strcat(prefix, s_num);
std::strcat(prefix, "\\");

and so on and so forth, I get a warning

warning C4996: 'strcat': This function or variable may be unsafe. Consider using strcat_s instead.

Why is strcat considered unsafe, and is there a way to get rid of this warning without using strcat_s?

Also, if the only way to get rid of the warning is to use strcat_s, how does it work (syntax-wise: apparently it does not take two arguments).

  • 13
    you should know that your example is horribly wrong. String constants (such as: char *s = "hello";) are NOT writeable. If you are lucky, it will crash, if you are unlucky it will make the application perform incorrectly in some subtle way. the proper way to do that you wanted is like this: char prefix[SIZE] = "Sector_Data\\sector"; where SIZE is big enough to hold the prefix AND whatever you intend to add to it. – Evan Teran Jun 01 '09 at 20:16
  • 3
    As written, your example contains a buffer overflow. Not just a possible buffer overflow, but a certain one. – Dietrich Epp Jun 01 '09 at 21:28

7 Answers7

29

If you are using c++, why not avoid the whole mess and use std::string. The same example without any errors would look like this:

std::string prefix = "Sector_Data\\sector";
prefix += "0";
prefix += "\\"

no need to worry about buffer sizes and all that stuff. And if you have an API which takes a const char *, you can just use the .c_str() member;

some_c_api(prefix.c_str());
Evan Teran
  • 87,561
  • 32
  • 179
  • 238
27

Because the buffer, prefix, could have less space than you are copying into it, causing a buffer overrun. Therefore, a hacker could pass in a specially crafted string which overwrites the return address or other critical memory and start executing code in the context of your program.

strcat_s solves this by forcing you to pass in the length of the buffer into which you are copying the string; it will truncate the string if necessary to make sure that the buffer is not overrun.

google strcat_s to see precisely how to use it.

Drew Hoskins
  • 4,168
  • 20
  • 23
  • 1
    strcat_s is one of a library of functions release as part of the Security Enhancements to the CRT -- http://msdn.microsoft.com/en-us/library/8ef0s5kh(VS.80).aspx – Sean Jun 01 '09 at 20:19
  • 4
    Truth being told, this is merely fearmongering on behalf of Microsoft in order to make sure that code that you write cannot be compiled anywhere but on Windows. `strcat_s`is nothing but a proprietary `strncat` (which has been around for 40 years). Further, `strcat` is not unsafe as such in the first place, nor is calling `strcat_s` with the length supplied by `strlen` any safer. What's unsafe is blindly accepting arbitrary (and arbitrary-length) user input, but using a non-standard function does not solve that issue, it will only crash in a different function (`strlen`). – Damon Apr 26 '11 at 17:06
  • 1
    @DeadMG: It is true that using `strcat_s` means you need not subtract the initial length from the total size, but other than that it is exactly the same, except it is non-standard and non-portable. But my point in general is that using "safe" functions does not make anything safer at all. You either know that you can trust some data (such as string literals in your program) or you know you can't trust it (user entered data), and in any case you must know your target buffer sizes. If that is not the case, all is lost, no matter what "safe" functions one uses. The same holds for strncat too, ... – Damon Apr 26 '11 at 17:40
  • 1
    ... as well as similar functions. I've seen people use `strncpy` to make their code safe, because they were told `strcpy` is so totally unsafe, and they ended up calling `strlen` so they knew how much memory to allocate and how much to copy. Which, of course, made the entire endeavour silly. Which is what I'm saying: not the functions must be safe, the concept must be. – Damon Apr 26 '11 at 17:44
  • 1
    @Damon: In addition, all those "safe" functions are extra-unsafe, because they abort execution or they don't, dependent on process-global state, and you can neither safely change it for your own calls (race-conditions and other code (maybe executing in parallel) depending on a different setting), nor depend on it having a specific state, though if you use even a handful sources, you can just about depend on them having conflicting requirements for correctness. – Deduplicator Nov 15 '15 at 22:39
5

You can get rid of these warning by adding:

_CRT_SECURE_NO_WARNINGS

and

_SCL_SECURE_NO_WARNINGS

to your project's preprocessor definitions.

Dusty Campbell
  • 3,146
  • 31
  • 34
  • 2
    Yes, suppressing the warning message by using above flag is best choice if you want's to write portable code. using strcat_s may lead to non-portable code as this is specific to Microsoft compiler. – Mohit Thakur Sep 14 '15 at 03:58
4

Because it has no means of checking to see if the destination string (prefix) in your case will be written past its bounds. strcat essentially works by looping, copying byte-by-byte the source string into the destination. Its stops when it sees a value "0" (notated by '\0') called a null terminal. Since C has no built in bounds checking, and the dest str is just a place in memory, strcat will continue going ad-infinidium even if it blows past the source str or the dest. str doesn't have a null terminal.

The solutions above are platform-specific to your windows environment. If you want something platform independent, you have to wrangle with strncat:

strncat(char* dest, const char* src, size_t count)

This is another option when used intelligently. You can use count to specify the max number of characters to copy. To do this, you have to figure out how much space is available in dest (how much you allocated - strlen(dest)) and pass that as count.

Doug T.
  • 64,223
  • 27
  • 138
  • 202
  • 1
    Even strncat is not secure. From MSDN: strncat does not check for sufficient space in strDest; it is therefore a potential cause of buffer overruns. Keep in mind that count limits the number of characters appended; it is not a limit on the size of strDest. See the example below. For more information, see Avoiding Buffer Overruns. – Drew Hoskins Jun 01 '09 at 20:06
  • Checking for sufficient space in the destination buffer is outside the scope of the language and requires additional facilities by the OS/compiler. – Doug T. Jun 01 '09 at 20:12
  • 2
    MSDN makes no sense about strncat(). It forces the programmer to enter a size, and two sizes can be gotten wrong just as easily as one. The problem with strncat(), like strncpy(), is that it doesn't do the same thing as a limited-length strcat() (or strcpy()). – David Thornley Jun 01 '09 at 20:41
4

That's one of the string-manipulation functions in C/C++ that can lead to buffer overrun errors.

The problem is that the function doesn't know what the size of the buffers are. From the MSDN documentation:

The first argument, strDestination, must be large enough to hold the current strDestination and strSource combined and a closing '\0'; otherwise, a buffer overrun can occur.

strcat_s takes an extra argument telling it the size of the buffer. This allows it to validate the sizes before doing the concat, and will prevent overruns. See http://msdn.microsoft.com/en-us/library/d45bbxx4.aspx

Herms
  • 37,540
  • 12
  • 78
  • 101
  • Now if the argument for `strcat_s` was also checked for correctness by the compiler, or the behavior of `strcat_s` was dependable on error, ... Unfortunately, neither is the case, so it's an exercise in futility. – Deduplicator Nov 15 '15 at 22:42
4

To turn the warning off, you can do this.

#pragma warning(disable:4996)

btw, I strongly recommend that you use strcat_s().

young
  • 2,163
  • 12
  • 19
0

There are two problems with strcat. First, you have to do all your validation outside the function, doing work that is almost the same as the function:

if(pDest+strlen(pDest)+strlen(pScr) < destSize)

You have to walk down the entire length of both strings just to make sure it will fit, before walking down their entire length AGAIN to do the copy. Because of this, many programmers will simply assume that it will fit and skip the test. Even worse, it may be that when the code is first written it is GUARANTEED to fit, but when someone adds another strcat, or changes a buffer size or constant somewhere else in the program, you now have issues.

The other problem is if pSrc and pDst overlap. Depending on your compiler, strcat may very well be simple loop that checks a character at a time for a 0 in pSrc. If pDst overwrites that 0, then you will get into a loop that will run until your program crashes.

Dolphin
  • 4,655
  • 1
  • 30
  • 25