I got a problem when reading SSL response that causes a segmentation fault. I read the response into a buffer, then append it to a malloced string and memory reset it to 0 till the response is fully read, but when I try this in a multi threaded program, after some operations it gives me segmentation fault. When I remove strcat it doesn't give me segmentation fault even if I run it for hours.

Example:

char* response = malloc(10000);
char buffer[10000] = { 0 };

while(SSL_read(ssl,buf,sizeof(buffer)) > 0){
    strcat(response,buffer);
    memset(buffer,0,sizeof(buffer));
}

Errors

free(): invalid next size (normal)
malloc_consolidate(): invalid chunk

I made sure of freeing both of SSL and CTX and close socket and free the malloced string.

CodePudding user response：

There are a few problems with your code:

1. You are not dealing with C strings, you are dealing with arbitrary byte sequences. SSL_read() reads bytes, not C strings, and you cannot treat them as strings. What you read cannot be assumed to be NUL-terminated (\0), therefore you cannot use strcat, strlen or other similar functions that operate on strings.

2. You are reading data continuously in a loop into a fixed size buffer. Your code will overflow the destination buffer (response) very easily.

3. Not an error, but there isn't really any need for an intermediate buffer to begin with. Why waste time copying stuff around two times (one with SSL_read() and one with strcat)? Just read directly into response. On top of that, the memset() to clear the contents of buffer is also unneeded. Overall you are scanning 3 times over the same buffer while you could simply do that once.

4. Again, not an error, but SSL_read() returns int and uses that to return the read size. You would be much better off using size_t to avoid unwanted problems with signed math and possible overflows. You can use SSL_read_ex() for this purpose.

Here's a snippet of code that does what you want in a more robust way:

#define CHUNK_SIZE 10000

unsigned char *response = NULL;
size_t size = 0;
size_t space_left = 0;
size_t total_read = 0;
size_t n;

while (1) {
    // Allocate more memory if needed.
    if (space_left < CHUNK_SIZE) {
        unsigned char *tmp = realloc(response, size   CHUNK_SIZE);
        if (!tmp) {
            // Handle realloc error
            break;
        }

        response = tmp;
        size  = CHUNK_SIZE;
    }

    if (SSL_read_ex(ssl, response   total_read, space_left, &n)) {
        total_read  = n;
        space_left -= n;
    } else {
        // Handle error
        break;
    }
}

CodePudding user response：

You never initialized response(). The arguments to strcat() have to be null-terminated strings.

You should also subtract 1 from the size of the buffer when calling SSL_read(), to ensure there will always be room for its null terminator.

char* response = malloc(10000);
response[0] = '\0';
char buffer[10000] = { 0 };

while(SSL_read(ssl,buf,sizeof(buffer)-1) > 0){
    strcat(response,buffer);
    memset(buffer,0,sizeof(buffer));
}