csocketstcpoverflow

stack smashing detected on function return

I have function for reading TCP packet of unknown size. The function uses MSG_PEEK to find the required size for the received JSON message (the \"} in the loop condition) and then runs the loop one more time without MSG_PEEK to clean the buffer. For some reason, the two prints at the end of the function work perfectly fine, but when I call the fn, my program dies instantly after the return and prints "*** stack smashing detected ***: terminated". The content_buffer shouldn't overflow, cause I'm allocating content_size + 1 and passing content_size into the recv right?

char *read_socket_raw(int socket)
{
    if (socket == -1)
    {
        fprintf(stderr, "Reading socket failed.\n");
        return NULL;
    }

    char *content_buffer = NULL;
    unsigned short content_size;

    why2_bool empty_buffer = 0; //WHETHER MSG_PEEK SHOULD BE USER OR NOT

    do
    {
        //FIND THE SENT SIZE
        content_size = 0;
        if (ioctl(socket, FIONREAD, &content_size) < 0 || content_size <= 0) continue;

        //ALLOCATE
        content_buffer = realloc(content_buffer, content_size + 1);

        read_section:

        //READ JSON MESSAGE
        if (recv(socket, content_buffer, content_size, !empty_buffer ? MSG_PEEK : 0) != content_size) //READ THE MESSAGE BY CHARACTERS
        {
            fprintf(stderr, "Socket probably read wrongly!\n");
        }

        if (empty_buffer) goto return_section; //STOP LOOPING
    } while (content_buffer == NULL || strncmp(content_buffer + (content_size - 2), "\"}", 2) != 0);

    //REMOVE JUNK FROM BUFFER (CUZ THE MSG_PEEK FLAG)
    empty_buffer = 1;
    goto read_section;

    return_section:

    content_buffer[content_size] = '\0';

    printf("'%s'\n", content_buffer);
    printf("'%p'\n\n", content_buffer);

    return content_buffer;
}
Solution

  • There is much to dislike about your function, but the one thing that seems downright incorrect is the type of variable content_size. The FIONREAD ioctl expects a pointer to int, but you've provided a pointer to unsigned short, instead. Undefined behavior results.

    In practice, your unsigned short is likely smaller than int, in which case ioctl() will presumably attempt to write past the bounds of that object. This is just the sort of thing that might trigger a stack smashing message and crash, for if the storage for that variable is at the beginning (top) of the function's stack frame, then overrunning its bounds will indeed smash the stack.

    Other things not to like: