Windows strcpy overflow question

[ekhule]

Hello,

I have the following code:

char buf[5];
strcpy(buf, "12345678");


When I run this from the IDE in Release mode, it works fine, no crashes. However, when I change "12345678" to "123456789" then I get this compilation warning:

warning C4789: destination of memory copy is too small

And also it crashes when i run it.

Can anyone explain why this is inconsistent, or what I am misunderstand?

Regards,
EK

[laserlight]

my guess is that it is also copying the null terminator?

The standard behaviour of memcpy involves copying the null terminator.

Can anyone explain why this is inconsistent, or what I am misunderstand?

You are in the realm of undefined behaviour. Don't do this, then you don't have to misunderstand.

[2kaud]

Your definition of buffer

char buffer[5];

allocates space for 4 characters plus the terminating null character (0). Therefore the maximum length of a string that should be copied to buffer is 4. Standard strcpy (as far as I am aware) copies starting from the memory location pointed by the source pointer to the memory location pointed by the destination pointer. This copy continues character by character until a null character is found in the source. strcpy does not know how much memory has been allocated for the destination and continues copying beyond the end of the allocated destination memory if a null has not been found in the source. Effectively strcpy performs

while (*d++ = *s++);

In your case all your inputs including "12345" have too many characters to be copied correctly and safely to the destination (buffer - maximum number 4 plus the terminating null). Input of "1234" is OK but "12345" is not.

char buffer[5];
strcpy(buffer, argv[1]);

char buffer[5];
strcpy(buffer, "12345678");

When copying exceeds the memory allocated it overwrites whatever happens to be in memory following buffer. This could be other data, code etc. This is a prime example of what is called 'buffer overflow' where the contents of a buffer has 'overflowed' into adjacent memory. This is a prime source of security issues allowing unauthorised code to be executed by carefully crafted input.

To make sure that buffer overflow does not occur, the new string safe functions should be used. These are the StringCch functions . If you must use the unsafe strcpy, strcat, gets etc functions then you need to make sure that the memory allocated for the destination is large enough to hold what is going to be copied to it.

In your case you are probably overwriting code so that is why your program is crashing.

[Eri523]

To make sure that buffer overflow does not occur, the new string safe functions should be used. These are the StringCch functions . [...]

Right, however, this is Win32 API. To avoid dependency on Win32, strcpy_s() from the C++ runtime library can be used as an alternative. It's still MS-specific (AFAIK), however.

[JohnW@Wessex]

I'm assuming that you're asking this question to understand legacy C code.

As you are using C++, the equivelent code in your first example would be.

#include <iostream>
#include <string>

using namespace std;

int main(int argc, char *argv[])
{
    string buffer(argv[1]);
    cout << "buffer content= " << buffer << "\n";
    cout << "String constructor executed\n";
}

No buffer overruns are possible.

[negative zero]

Right, however, this is Win32 API. To avoid dependency on Win32, strcpy_s() from the C++ runtime library can be used as an alternative. It's still MS-specific (AFAIK), however.

Yes, it's an MS-specific recommendation that never made it to the new C11 standard. The C standard has had strncpy for years. Use that, if you desire fixed length fields... Just be aware that unlike the non-portable strcpy_s, strncpy might not append a null terminator to the end of your string. If you want that guarantee, end your fixed length field one byte earlier and put the terminator there yourself.

#include <string.h>
int main(void) {
    char fubar[6];
    strncpy(fubar, "hello world", sizeof (fubar) - 1);
    fubar[sizeof (fubar) - 1] = '\0';
    printf("fubar contains: %s\n", fubar);
    return 0;
}

[Paul McKenzie]

Can anyone explain why this is inconsistent, or what I am misunderstand?

Here is what you're misunderstanding: you're using C++.

When you overwrite memory like this in C++, there is no guarantee how your program will behave. Unlike other computer languages, with C++ (and C) you do not get an automatic "crash" or "message box" or some other error indicator when you do these "tricks" at runtime.

You can do all sorts of illegal (but compilable) things in C++, and all seems to work -- until you try to run that program somewhere else, or you run the program at a different time of day, or you run that program on a customer's machine, or you change a couple of lines of code that are totally benign, but the program then crashes.

Haven't you heard of programs that run perfectly in-house, and then when you go take that program to the customer site, things crash? It's situations just like this that causes those issues.

C++ has something called undefined behaviour, meaning exactly that -- you write code that does something that isn't defined as to what will happen, and guess what? You don't know what will happen.

Regards,

Paul McKenzie

[negative zero]

I can stick a knife in the toaster and there is a chance I won't have to go to hospital...