Possible Buffer Overrun

Question

I'm having an infuriating issue here where I'm crashing on malloc/calloc/strdup and I'm assuming currently that it's because of a buffer over run somewhere.

I'm finding this very difficult to find and I was wondering if any of you can offer me a hand. I'll post code snippets here, and link to full source.

File reading and array operations: (common.c)

Pastebin

char * S6_ReadFileBytes(const char* path)
    FILE * file;
    long length;
    char * bytes = NULL;
    file = fopen(path, "r");
    fseek(file, 0, SEEK_END)
    length = ftell(file);
    fseek(file, 0, 0);
    bytes = (char*)calloc(1, (size_t)length + 1);
    fread(bytes, 1, (size_t)length, file);
    return bytes;

S6_Array * S6_ArrayNew(size_t count, size_t typeSize)
    S6_Array * a = (S6_Array*)malloc(sizeof(S6_Array));
    a->typeSize = typeSize;
    a->Length = count;

void * S6_ArrayGet(S6_Array * a, int idx)
    return &((char*)a->Data)[idx * a->typeSize];

void S6_ArraySet(S6_Array * a, int idx, void * val)
    memcpy(&((char*)a->Data)[idx * a->typeSize], val, a->typeSize);

void S6_ArrayGrow(S6_Array * a, int amount)
    void * data;
    data = realloc(a->Data, (a->Length + amount) * a->typeSize);
    a->Data = data;
    a->Length += amount;

void S6_ArrayPushBack(S6_Array * a, void* val)
    S6_ArrayGrow(a, 1);
    S6_ArraySet(a, a->Length - 1, val);

CSV Reading: (CSV.c)

Pastebin

void S6_CSV_PushRect(S6_Array ** rectangles, S6_Rectangle * rect)
    if( !*rectangles )
        *rectangles = S6_ArrayNew(1, sizeof(S6_Rectangle*));
        S6_ArraySet(*rectangles, 0, &rect);
    else
        S6_ArrayPushBack(*rectangles, &rect);

int S6_CSV_ReadRects(const char* file, S6_Array ** rectangles)
    char * bytes = S6_ReadFileBytes(file);
    char * line;
    char * nameIndex;
    size_t nameLength;
    S6_Rectangle * tempRect;

    line = strtok( bytes , "\n");
    while( line )
        nameIndex = strstr(line, ",");
        tempRect = (S6_Rectangle*)calloc(1, sizeof(S6_Rectangle));

        nameLength = (size_t)(nameIndex - line) + 1;
        strncpy(tempRect->name, line, nameLength-1);
        tempRect->name[nameLength-1] = '\0';

        sscanf(nameIndex, "%*[,]%d%*[,]%d%*[,]%d%*[,]%d", &tempRect->x, &tempRect->y, &tempRect->w, &tempRect->h)

        S6_CSV_PushRect(rectangles , tempRect);
        strtok(NULL, "\n");
    free(bytes);

A function where I modify the array: (BinPacker.c)

Pastebin

int S6_BinPacker_Pack(S6_Array * rectangles, int binSize)
    // This sort appears to be working fine. View pastebin for test.
    qsort(rectangles->Data, rectangles->Length, sizeof(S6_Rectangle*), S6_BinPacker_CompareRects);

CSV Writing [CRASH] : (CSV.c)

Pastebin

void S6_CSV_WriteRects(const char* file, S6_Array * rectangles)
    char * bytes = NULL;
    char buffer[128];
    S6_Rectangle * tempRect;
    size_t i;

    for( i = 0; i < rectangles->Length; ++i)
        tempRect = *(S6_Rectangle**)S6_ArrayGet(rectangles, i);
        memset(buffer, '\0', sizeof(buffer));

        sprintf(buffer, 
            "%s,%d,%d,%d,%d\n",
            tempRect->name,
            temprect->x,
            temprect->y,
            temprect->w,
            temprect->h);
        if( bytes )
            bytes = strcat(bytes, _strdup(buffer));
        else
            bytes = _strdup(buffer);

So I'm crashing here on the strcat(bytes, _strdup(buffer)) line. When I separate it out It's still the string duplication or any sort of allocation I've tried.

I get the following break dialog from visual studio:

Windows has triggered a breakpoint in myapp.exe.

This may be due to a corruption of the heap, which indicates a bug in Slant6.Debug.exe or any of the DLLs it has loaded.
This may also be due to the user pressing F12 while Slant6.Debug.exe has focus.
The output window may have more diagnostic information.

And the break point it triggers is in tidtable.c on

PFLS_GETVALUE_FUNCTION flsGetValue = FLS_GETVALUE;

SOLUTION

strdup doesn't do any allocations, and even if it did I would be leaking like crazy. So instead of:

bytes = strcat(bytes, _strdup(buffer));

in CSV.c, I replaced it with some manual string concatenation that's easier for me to read (and remember).

size_t oldSize = strlen(bytes);
size_t bufferSize = strlen(buffer);
size_t newSize = oldSize + bufferSize ;

char * newMem = (char*)calloc(newSize + 1, 1);

memcpy(newMem, bytes, newSize);
memcpy(&newMem[oldSize], buffer, bufferSize);

free(bytes);
bytes = newMem;

/SOLUTION

Answer 1

I'm thinking that this line:

bytes = strcat(bytes, _strdup(buffer));

Does not do what you think it does.

You are making a copy of a string (buffer), and then concatenating that onto bytes. The duplicated string is never freed and bytes is only as big as the last _strdup, thus doing a strcat will overflow the buffer.

You need to allocate (or reallocate) strlen(bytes) + strlen(buffer), etc. etc. for the strcat.