1
    char* buf = (char*)malloc(sizeof(char) * 11);
    for (int i = 0; i < 5; i++)
        buf[i] = '?';
    strcpy_s(buf + 5, sizeof(char) * 11, "apple");
    buf[10] = '\0';
    printf(buf);
    free(buf);

I have the code above. It says "heap corruction detected" when buf is freed. I want to use strcpy_s to write to the last 5 characters of buf. From what I read and understood, the size argument should be the size of the destination buffer but I also tried doing

strcpy_s(buf + 5, 5, "apple");

But in this case it says buffer is too small. What should I do?

Cool_Cornflakes
  • 315
  • 2
  • 13
  • Two things: First you [should not cast the result of `malloc`](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858); Secondly the result of `sizeof(char)` is specified to *always* be `1`, so you never need to multiply with it. – Some programmer dude Dec 31 '22 at 12:15
  • 1
    As for your code, what's wrong with `strcpy(buf, "?????apple")`? Or possibly if the `"apple"` part is from user.input, `snprint(buf, 11, "?????%s", "apple")`? And for any string containing user-input, don't pass it directly to any `printf` function as a format string. What happens if the user input e.g. `%s`? Either use `printf("%s", buf)` or `puts(buf)`. – Some programmer dude Dec 31 '22 at 12:17
  • 1
    With `strcpy_s(buf + 5, 5, "apple");`, the destination buffer needs **six** bytes, not five. That doesn't explain any heap corruption errors, however. And just use `strcpy()`. `strcpy()` is **NOT** "deprecated", and `strcpy_s()` isn't really more secure. Read this: [**Field Experience With Annex K — Bounds Checking Interfaces**](https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm) – Andrew Henle Dec 31 '22 at 12:23

1 Answers1

4

strcpy_s(buf + 5, sizeof(char) * 11, "apple"); tells strcpy_s there are 11 bytes available at buf + 5. That is false; 11 bytes were reserved at the location where buf points. Starting from buf + 5, there are only 6 bytes available.

This strcpy_s overruns the array and writes into other memory, corrupting the so-called heap. The reason for this is that, although one might think strcpy_s copies individual bytes until a terminating null character is seen, the specification for the routine allows it to use the entire destination buffer regardless of the length of the source string. Specifically, C 2018 K.3.7.1.4 5 says:

All elements following the terminating null character (if any) written by strncpy_s in the array of s1max characters pointed to by s1 take unspecified values when strncpy_s returns a nonzero value.

The purpose of that is to allow an optimized implementation of strncpy_s to copy large groups of data than individual bytes.

(One might wonder how one can avoid overrunning the terminating null in the source array when copying such groups of data. An answer for this is that, in common C implementations, it is safe to copy aligned words because the memory protections operate in page-aligned units. So a strcpy_s implementation may copy whole aligned words from the source to the destination as long as they fit into the destination and no null character has yet been seen in a word.)

Eric Postpischil
  • 195,579
  • 13
  • 168
  • 312
  • 1
    *This `strcpy_s` overruns the array and writes into other memory* ROFLMAO. So much for "more secure". – Andrew Henle Dec 31 '22 at 12:32
  • 3
    @AndrewHenle: Routines cannot operate correctly when given incorrect arguments. – Eric Postpischil Dec 31 '22 at 12:34
  • That's exactly my point. There's no way to give "more correct" arguments to `strcpy_s()` than there is to `strcpy()`. Or any other of the Annex K functions. The only useful addition is `memset_s()` which cannot be elided, making it actually useful for clearing memory. – Andrew Henle Dec 31 '22 at 12:38
  • @AndrewHenle without strcpy_s it is not possible to copy a string safely to a buffer that is smaller than the string (unless you write code which imitates strcpy_s). Moreover, strcpy_s is able to deal with non-zero terminated source char[] correctly. So its use seems very clear to me? You complaint seems to be "If I enter garbage into strcpy_s, weird things happen!", which is not suprising to me. – Cheiron Dec 31 '22 at 13:18
  • @Cheiron *without strcpy_s it is not possible to copy a string safely to a buffer that is smaller than the string* What?!?! Seriously? [This can be used **safely**](https://port70.net/~nsz/c/c11/n1570.html#7.24.3.2). So can [this](https://port70.net/~nsz/c/c11/n1570.html#7.21.6.5). As well as [this](https://port70.net/~nsz/c/c11/n1570.html#7.24.2.1). You can even use [this](https://port70.net/~nsz/c/c11/n1570.html#7.24.2.4) **with complete safety** by explicitly adding the terminating `NUL`. – Andrew Henle Dec 31 '22 at 13:28
  • @Cheiron Nevermind the entirety of the Annex K functions have signficant flaws, including [being multithread-**unsafe**](https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1866.htm) and [*de facto* non-portable](https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1967.htm#impementations): "Microsoft Visual Studio implements an early version of the APIs. However, the implementation is incomplete and conforms neither to C11 nor to the original TR 24731-1. ... As a result of the numerous deviations from the specification the Microsoft implementation cannot be considered conforming or portable." – Andrew Henle Dec 31 '22 at 13:37
  • @AndrewHenle Ok, point taken. Still though, I do not understand why you consider it a flaw of strcpy_s that it does not perform as expected when handed garbage? – Cheiron Dec 31 '22 at 13:47
  • @Cheiron The entire point of the `*_s()` functions is that they're supposed to be *safer*. Yet here we see `strcpy_s()` overrunning a buffer when called with `strcpy_s(buf + 5, sizeof(char) * 11, "apple");`, even though there are enough bytes available in `buf` to hold the six that need to be copied. Someone looking at that code could very well think it's a safe call because there's only 6 bytes of data to be copied. Nope - as stated in this answer, `strcpy_s()` winds up being **less** safe than a simple `strcpy()` would be in this case. And it's allowed to be per the standard. – Andrew Henle Dec 31 '22 at 13:55
  • @Cheiron (cont) The bottom line is, if you don't know what you're doing with C strings, there's nothing that can save you. The default Annex K response to bad input is to summarily kill the process, which is not acceptable in many environments. (And you can't safely change that behavior in multithreaded programs, or in libraries supplied for general use). So if you have to write code to run under such conditions, you have to get it all correct anyway (and today there are lots of tools to help with that - unlike when these functions were created). And once you do that, Annex K adds nothing. – Andrew Henle Dec 31 '22 at 14:01