fgets() crashs after a number of executions

Question

I'm coding a program to crack the CRC16. I've been having some problems with outputting the file and keep the calculated CRC16(have no idea why it changes when I write it to a file). So what I'm doing here is read the input file, writing it to a output file with some gibberish and then I read the output file again and calculate it's CRC16. If it matches with the desired CRC16, then it is done. However after a bunch of executions the fgets() method crashes with a Seg fault.

Anyone could help me? Please ignore the performance issues, this is a test.

int main(int argc, char* argv[]){

        char outfile[strlen(argv[1])];
        strcpy(outfile,argv[1]);

        strcat(outfile,".crack");

        char crc16[5];
        strcpy(crc16,argv[2]);
        char newcrc16[5];
        char gebrish[80];
        char cat[2];
        int full = 1;
        int p = 0;
        int i,j,k;


        for(i=32; i< 128;i++)
                for(j=32; j< 128; j++)
                        for(k=32; k < 128; k++){
                                gebrish[0] =i;
                                gebrish[1] =j;
                                gebrish[2] =k;
                                gebrish[3] = '\n';
                                gebrish[4] ='\0';

                                boost::crc_16_type result;

                                FILE* file;
                                FILE* out;
                                char line[100];

                                printf("read out\n");
                                out = fopen(outfile,"w");

                                printf("read file\n");
                                file = fopen(argv[1],"r");
                printf("wrt\n");
                                while(fgets(line,80,file) != NULL){
                                        fputs(line,out);
                                }
                                fputs(gebrish,out);

                                fclose(file);
                                fclose(out);

                                printf("read gain\n");
                                out = fopen(outfile,"r");

                                while(fgets(line,80,out) != NULL){
                                        result.process_bytes(line,strlen(line));
                                        printf("%s",line);
                                }

                                int crc = result.checksum();

                                sprintf(newcrc16,"%x",crc);
                                printf("%s",newcrc16);

                                if(strcmp(crc16,newcrc16) == 0){

                                        printf("%s",gebrish);
                                        return 0;

                                }
                        }



        return 0;
}

Answer 1

The problem is that I tried to Read and Write from the same file in different moments without calling fclose() after the use. This way after some execution of the loop it crashes with a STATUS_VIOLATION. I have no idea why it didn't crash right away, but all I did was add a flcose() after reading the file for the CRC16 calculation.

Answer 2

This causes a buffer overrun:

char outfile[strlen(argv[1])];
strcpy(outfile,argv[1]);

strcat(outfile,".crack");

as there is not enough space in outfile for terminating null character and ".crack". It will be overwriting memory it is not supposed to and may be the cause of the segmentation fault.

Change to:

    char outfile[strlen(argv[1]) + 1 + 6];
    strcpy(outfile,argv[1]);

    strcat(outfile,".crack");

Before accessing argv elements ensure they have been provided by checking argc:

if (argc > 2)
{
    /* Safe to use argv[1] and argv[2]. */
}

Check return values from fopen() also.

Answer 3

The error is most likely due to not checking the return value from open, and then calling fgets on a bad file. Returns from system calls should always be checked if subsequent operations depend on them. Even close can fail.