getting segmentation fault around strlen() when doing .mp3 files

Question

Whenever I try and give a file like /home/..../TulsaQueen.mp3 the program gives a segmentation fault. gdb says it has something to do with strlen() but I cannot figure it out. why is it happening? I am creating a server and need to handle GET request my .jpg works everytime but .mp3 doesn't.

void *connectionThread(void *socket_desc){

    char buffer[1024];
    int newsockfd = *(int*)socket_desc;
    int n;
    magic_t myt = magic_open(MAGIC_ERROR|MAGIC_MIME_TYPE);
    magic_load(myt,NULL);
    bzero(buffer,256);
    FILE * picture;
    int size;
    char *str_size = malloc(100);
    struct stat sb;
    int fd = open("TulsaQueen.mp3", O_RDONLY );
    fstat( fd, &sb );

 while (1)
     {memset(buffer, 0, 1024);n = read(newsockfd,buffer,1024);
     if (n < 0){ printf("ERROR reading from socket"); close(newsockfd);pthread_exit(NULL);}
     printf("%s",buffer);


    char *token = strtok(buffer," ");
    if(token != NULL)
    token = strtok(NULL, " ");

    printf("%s\n",token);

    if(strcmp(magic_file(myt,token),"audio/mpeg") == 0){

        write(newsockfd,"HTTP/1.1 200 OK\r\n",strlen("HTTP/1.1 200 OK\r\n"));
        write(newsockfd,"Content-Length: ",strlen("Content-Length: ")); 
        snprintf(str_size,100,"%li",sb.st_size);
        write(newsockfd,str_size,strlen(str_size));
        write(newsockfd,"\r\n",strlen("\r\n"));
        write(newsockfd,"Content-Type: ",strlen("Content-type: "));
        printf("magic output: '%s'\n",magic_file(myt,token));
        write(newsockfd,magic_file(myt,token),strlen(magic_file(myt,token))); 
        write(newsockfd,"\r\nConnection: keep-alive",strlen("\r\nConnection: keep-alive"));
        write(newsockfd,"\r\n\r\n",strlen("\r\n\r\n"));

        write( newsockfd, &sb.st_size, sizeof( sb.st_size ) );
        sendfile( newsockfd, fd, 0, sb.st_size );
    }
    if(strcmp(magic_file(myt,token),"image/jpeg") == 0){
    picture = fopen (token, "r");
    fseek(picture,0,SEEK_END);
    size = ftell(picture);
    fseek(picture,0,SEEK_SET);
    char file_buf[size];
    snprintf(str_size,16,"%d",size);

    //Header for HTTP standards
    write(newsockfd,"HTTP/1.1 200 OK\r\n",strlen("HTTP/1.1 200 OK\r\n"));
    write(newsockfd,"Content-Length: ",strlen("Content-Length: "));
    write(newsockfd,str_size,strlen(str_size));
    write(newsockfd,"\r\n",strlen("\r\n"));
    write(newsockfd,"Content-Type: ",strlen("Content-type: "));
    write(newsockfd,magic_file(myt,token),strlen(magic_file(myt,token))); //get Content-type
    write(newsockfd,"\r\nConnection: keep-alive",strlen("\r\nConnection: keep-alive"));
    write(newsockfd,"\r\n\r\n",strlen("\r\n\r\n"));

    rewind(picture);
    while(!feof(picture)){
    fread(file_buf, sizeof(char), sizeof(file_buf), picture);
        write(newsockfd, file_buf, sizeof(file_buf));
    bzero(file_buf, sizeof(file_buf));
    }
    }

     }  
    close(fd);
    free(buffer);
    fclose(picture);
    close(newsockfd);
    magic_close(myt);
    pthread_exit(NULL);
    return 0;
}

Here is the error I got

Thread 2 "a.out" received signal SIGSEGV, Segmentation fault. (gdb) where (0) strlen () at ../sysdeps/x86_64/strlen.S:106 (1) 0x00007ffff764069c in _IO_puts (str=0x0) at ioputs.c:35 (2) 0x0000000000401623 in connectionThread () (3) 0x00007ffff7bc16ba in start_thread (arg=0x7ffff73b6700) at pthread_create.c:333 (4) 0x00007ffff76d782d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109

Answer 1

One error in the code, that might explain the issue, has to do with the buffer size, memset and read...

The buffer is exactly 1024 bytes long. You set all these bytes to 0 and then you read up to 1024 bytes.

But, what happens when you read exactly 1024 bytes without any NUL (0) bytes at the end? ...

... if this happens than strlen will overflow while searching for the NUL byte (on byte 1025 and onwards), resulting is a segmentation fault.

One way to resolve this specific issue would be to read 1023 bytes... i.e.:

memset(buffer, 0, 1024);
n = read(newsockfd,buffer,1023);

A better way to resolve this specific issue (as suggested in the comments) would be to use n to set the NUL byte value. i.e.:

n = read(newsockfd,buffer,1023);
if (n < 0) {
   // ...
}
buffer[n] = 0;

This way you make sure a NUL byte is always present within the allocated buffer.

I should point out that tokenizing the string with strtok, as well as using str* functions to parse the incoming data, is somewhat ineffective and might produce unexpected errors / results...

...especially when you're in a multi-threaded environment...

For example, strtok is not thread-safe... and even though there are thread-safe variations, it's hardly ideal.

Moreover, it might corrupt incoming data unintentionally - after all, you're only using the first "token", so I assume you expect the rest of the data to remain unaltered.

Good Luck!

P.S.

• strlen(magic_file(myt,token)) - what if magic_file(myt,token) is an invalid string (i.e. NULL)?

• strlen("\r\n\r\n") is always 4... why do you use a function call for this?

• free(buffer) should probably have been free(str_size)