Invalid Read of size 1 on running Valgrind

Question

The following is the error that I got after running Valrind    
valgrind --tool=memcheck --leak-check=full --show-reachable=yes ./out



  ==12140== Invalid read of size 1
    ==12140==    at 0x4C2DF84: strncat (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==12140==    by 0x40100D: createSortedRuns (final.c:178)
    ==12140==    by 0x401297: main (final.c:249)
    ==12140==  Address 0x51fd549 is 0 bytes after a block of size 9 alloc'd
    ==12140==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==12140==    by 0x400EFD: createSortedRuns (final.c:150)
    ==12140==    by 0x401297: main (final.c:249)
    ==12140== 

The other error that I am getting is:

==12140== Invalid write of size 1
==12140==    at 0x4C2DFD0: strncat (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12140==    by 0x40100D: createSortedRuns (final.c:178)
==12140==    by 0x401297: main (final.c:249)
==12140==  Address 0x51fd54d is 4 bytes after a block of size 9 alloc'd
==12140==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12140==    by 0x400EFD: createSortedRuns (final.c:150)
==12140==    by 0x401297: main (final.c:249)
==12140== 
==12140== 
==12140== 250 errors in context 7 of 13:

The other error that I am getting is:

==12140== Invalid read of size 1
==12140==    at 0x4C2DF84: strncat (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12140==    by 0x40100D: createSortedRuns (final.c:178)
==12140==    by 0x401297: main (final.c:249)
==12140==  Address 0x51fd549 is 0 bytes after a block of size 9 alloc'd
==12140==    at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==12140==    by 0x400EFD: createSortedRuns (final.c:150)
==12140==    by 0x401297: main (final.c:249)
==12140== 

The following is the snippet of the program:

  void createSortedRuns(int start, int end, char file[]){

        FILE *givenFile = NULL;
        givenFile=fopen(file,"r+b");
        rewind(givenFile);
        int i=0;
        /*printf("File:%s\n",file);
        printf("File Length:%zu\n",strlen(file));*/
        char *temp = (char *)malloc(sizeof(char)*strlen(file));



        for(i=start; i<end; i++){
            fseek(givenFile,0,SEEK_CUR);
            fread(&inputBuffer[act_inputBufferSize],sizeof(int),1,givenFile);
            act_inputBufferSize++;

            if((act_inputBufferSize==max_inputBufferSize) ||(i==(end-1))){

                //sort
                qsort(inputBuffer,act_inputBufferSize,sizeof(int),compare);

                // write to file

                strncpy(temp,file,strlen(file));


                //int k=0;
                char counter[4]={0};
                /*for(k=0;k<4;k++){
                    counter[k]=0;
                }*/


                snprintf(counter,5, ".%03d", totalSortedFiles);
                counter[strlen(counter)] = '\0';
                strncat(temp,counter,4);

                FILE *int_File = NULL;
                int_File=fopen(temp,"w+b");
                rewind(int_File);
                fwrite(&inputBuffer,sizeof(int),act_inputBufferSize,int_File);

                // register the sub_fo information
                small_fo=register_sub_fo(small_fo,totalSortedFiles,temp, 0 ,act_inputBufferSize,act_inputBufferSize);


                // increase the totalSortedFiles
                totalSortedFiles+=1;


                // Reinitialize
                            int j=0;
                            for(j=0;j<max_inputBufferSize;j++){
                                inputBuffer[j]=0;
                            }
                            act_inputBufferSize=0;

                            int len = strlen(temp);
                            temp[len-4] = '\0';
               // Garbage
                            fclose(int_File);
                            int_File = NULL;



            }
        } //for_end
        fclose(givenFile);
        givenFile=NULL;

        free(temp);
        temp=NULL;


    } // func_end

Valgrind is showing that the error is at line "strncat(temp,counter,4);" Can someone please explain me the error ?

Answer 1

strncpy function does not zero-terminate the target buffer if the size limit was reached during copying. You are using strncpy in your code and it definitely reaches the size limit

strncpy(temp,file,strlen(file));

This means that at this point temp is not zero-terminated. temp is not a string after this strncpy call.

After that you do

strncat(temp,counter,4);

But strncat requires its first operand to be a string. In your case it is not a string. The behavior is undefined.

As it has been stated many times, strncpy is not a limited-length string copying function. Using it as such is error prone and lame. Even if it can be made to "work", it is still not the right tool for the job.

In addition to that, the buffer you allocated for temp cannot accommodate any more characters in addition to what is already there after that strncpy. The buffer is allocated as exactly strlen(file) characters long.

Also, this statement does not seem to make any sense at ll

counter[strlen(counter)] = '\0';

In order for strlen to work, your counter must already be properly zero-terminated. So, what you are doing is re-terminating the counter string at the exact location at which it is already terminated. What is the point of that?

You don't need to do that. snprintf already produced a zero-terminated string.

However, your snprintf invocation is problematic by itself.

char counter[4]={0};
snprintf(counter,5, ".%03d", totalSortedFiles);

Your counter array is declared with size 4. By passing 5 as the buffer size to snprintf you are lying to snprintf. In general case snprintf will go out of bounds. The behavior is undefined.

The format you used in snprintf suggests that you actually need a buffer of size 5.