How could this buffer be overrun?

Question

I apologize in advance for the useless title of this question, but nothing seemed to fit better.

The idea here is to replicate argv in another variable, essentially making a copy of it. So the basic idea of what the function does is, use malloc() to request some space for a copy and then iterate through argv making copies of each element.

This is the code I'm working with, the development environment is right now Visual Studio 2019 (even if it's not strictly a C compiler...):

// Returns a copy of an array of strings (intended for argv, but should work with any of them):
wchar_t** copyArgv(size_t argc, wchar_t* argv[]) {
    // Allocate space for the array of arguments:
    wchar_t** argsCopy = malloc(((argc + 1) * sizeof(wchar_t*)));
    if (!argsCopy)
        return NULL;
    // Copy each one of them:
    for (size_t i = 0; i < argc; i++) {
        argsCopy[i] = _wcsdup(argv[i]);
        if (!argsCopy[i]) {
            // Should also free any previous copied string I left that part out in the paste.
            free(argsCopy);
            return NULL;
        }
    }
    argsCopy[argc] = NULL;
    return argsCopy;
}

I've been trying different ways to make a copy of argv but each and everyone of them lets VS to believe there can be a buffer overrun when I make a copy of an argument (line:argsCopy[i] = _wcsdup(argv[i]);) or reading invalid data in the next line, meaning reading out of the bounds of the reserved space.

All of this has lead me to believe the problem lies in the (now) only malloc() call to reserve space for the array of arguments.

Yet I'm banging my head against the wall trying to figure out what the problem is, I mean, I think I'm asking for enough space.

I've tried other compilers as well, latest stable versions of Clang and GCC don't seem to show any such warning. So I decided to ask you, seasoned programmers, if you can spot the problem, or it's some sort of compiler bug (unlikely I bet).

For reference these are the exact warnings VS2019 is throwing (in a 64-bit compilation):

In the assignment:

Buffer overrun while writing to 'argsCopy': the writable size is '((argc+1))*sizeof(wchar_t *)' bytes, but '16' bytes might be written.

Next line, the test for NULL:

Reading invalid data from 'argsCopy': the readable size is '((argc+1))*sizeof(wchar_t *)' bytes, but '16' bytes may be read.

Answer 1

These are warnings from the static analyzer. It tries, for example, to recognize buffer overflow situations.

Warning

It is important to note that these are warnings and not error messages. The compiler says that there might be something potentially wrong. Static analysis is generally a difficult thing.

False Positive

There is no buffer overrun situation, so it is a false positive. I would assume, that this message disappears in a future update.

Change the Code a Bit

If we change the memory allocation line as follows:

wchar_t** argsCopy = (wchar_t**)calloc(argc + 1, sizeof(wchar_t*));

then there will be no more warnings from Visual Studio 2019.

The number of bytes allocated remains the same. However, the warnings disappear.

Test

Before the change the VS Error list looks like this:

After the application of my proposed changes, the warnings have disappeared:

Answer 2

I could be mistaken, however, after playing around with an online copy of visual studio (https://rextester.com/l/c_online_compiler_visual), I am forced to assume that you have forgotten to include string.h or wchar.h (either one works). Visual Studio appears to assume that your return type is an integer instead of a wchar_t* as the function is not defined. It seems that there is a bit of "magic" due to the fact that it is a reserved function starting with an _ and therefore it doesn't issue other warnings? Again without your exact environment though I am forced to partially speculate (your comment on your target changing the warning gave me a hopefully correct hint).

Answer 3

The key point might be you didn't malloc the enough space to hold the data you want to copy.

I don't know whether I actually understand what you want to do, I assume you want to copy two-dimensional character array to another memory segment and then return its address, and the array has 'argc' lines, the address of each line of string is stored in the array of argv.

But why did you use argc+1 instead of argc? For malloc extra space to prevent buffer overrun? and more important is sizeof(wchar_t*) will return the size of a pointer ( 8 bytes in 64 bits system ). It will not return the size of one of the string in the two-dimensional array we want.

Answer 4

1) One way to replicate argv is explained below but., 2) I am unable to understand why you want to make a replica of argv? what use-case/user-problems does it solve?

as I mentioned in (1), here is one of the ways, which is essentially about copying all the contents that argv has into your buffer. It goes something like this (PS: there could be compilation erros as I"m typing in my phone while being in a taxi, so don't have access to a hi quality C compiler to cross check)

int numArgc = argc
char** argvCopy;

for (i=0;i<argc,i++)
{

 argvCopy[i] = malloc(sizeof(char)*strlen(argv[i]));
 strcpy(argvCopy[i], argv[i]);

}

//please do not forget to Free this malloc'ed memory (a very common C programming error) //when you don't need it anymore 

please do tell the problem that you want to solve