Heap corruption error with delete []

Question

So I'm a pretty big new at C++, so I'm sure this is a relatively simple problem, but I have a legacy C++ app I'm trying to trace a heap corruption problem and have traced it to this function:

void LTrimZeros(CString *pstr)
{
    char *psz1;
    char *psz2;

    if ( pstr->GetLength() == 0 )
        return;


    psz1 = new char[pstr->GetLength() + 1];
    psz2 = psz1;

    strcpy_s( psz2, strlen(psz2), (const char *) *pstr );

    while ( *psz2 == '0' )
    {
        psz2++;
    }

    *pstr = psz2;

    delete [] psz1;

    return;
}

When it tries to delete psz1 it throws a heap corruption error. Again I am pretty new to C++, so I didn't want to try to fix this and accidentally introduce a memory leak, so I thought I'd ask the experts. Alternative solutions of the same function are also fine, as this app was written in like c++4 originally, but now is upgraded to c++11 (Also a brief explanation of why this causes heap corruption would help a lot).

Answer 1

strlen(psz2) is reading uninitialised memory so may read beyond the end of your array. This means that the length you pass to strcpy_s will be unpredictable and may result in you writing beyond the end of the memory allocated for psz1.

Assuming the end of your function is valid (I'm not sufficiently familiar with CString to say for sure), you could simply change your strcpy_s line to

strcpy_s( psz2, pstr->GetLength() + 1, (const char *) *pstr );

You may run into problems here with win32 string handling functions that switch between 8 and 16-bit characters depending on the UNICODE and _UNICODE defines. I agree with Alok Save and others that switching to using std::string would be clearer and simpler.

Answer 2

From MSDN:

errno_t strcpy_s(
   char *strDestination,
   size_t numberOfElements,
   const char *strSource 
);

Here, in your code, you are calling strlen on an uninitialized array, you need to fix it (pass the maximum number of elements the destination buffer can store):

strcpy_s( psz2, strlen(psz2), (const char *) *pstr );