How can I avoid a Segmentation fault in my Recover.c

Question

I have looked thoroughly through other solutions, but they tend to be specific to another person's code. I have also used Valgrind to try and isolate the problem, and considered that I may have to use free() and malloc() somewhere, but I have tried this and don't completely understand what they do anyway. Asking this is my last resort, but I feel I would only waste more time if I were to continue trial-and-error in my code. Here is my code:

/**
* recover.c
*
* Computer Science 50
* Problem Set 4
*
* Recovers JPEGs from a forensic image.
*/

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>

int main(int argc, char* argv[])
{
    int counter = -1;
    char* title = NULL;
    int buffer[512] = {0};
    FILE* infile = fopen("card.raw", "r");
    FILE* img = NULL;

    // skip over empty space in card
    for (;fread(buffer, 512, 1, infile) == 0;)
    {      
    }

    // read a block in the card
    for(;fread(buffer, 512, 1, infile) == 1;)
    {

        // if new jpg found
        if ((buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff) && (buffer[3] == 0xe0 || buffer[3] == 0xe1))
        {

            // if an image file is already open
            if (img != NULL)
            {
                fclose(img);
            }

            // create new file for the jpg
            counter++;
            if (counter < 10)
            {
                sprintf(title, "00%d.jpg", counter);
            }
            else
            {
                sprintf(title, "0%d.jpg", counter);
            }

            // open file to copy jpg into
            img = fopen(title, "a");

        }

        // if a new jpg is not found
        else
        {
            // write a block from one jpg into the output file
            fread(buffer, 512, 1, infile);
            fwrite(buffer, 512, 1, img);

        }
    }
    // process complete, close all files
    fclose(img);
    fclose(infile);
}

Any help is appreciated

Andy

Answer 1

There's a bunch of issues here: 1) You're using the wrong type for your buffer array. It should store bytes, not ints.

2) Your first for loop does nothing at all. It only calls fread once without ever checking if the signature is there. What would happen if there's a jpeg signature there?

3) sprintf doesn't allocate memory for the string it produces. You're declaring a pointer (char* title) and you set it to NULL and then you're trying to store a string there. First, you don't have any memory allocated for that. Second, you're trying to write to a NULL pointer, hence the segfault. Use a char array instead and give it enough room

4) When the signature is found, you never write that chunk into a file. You simply skip it, the loop will go back and only read and write the next block (if there's a jpeg open already, if there isn't where are you going to write?)

5) Don't use "a" as argument to fwrite, it should be "w".

And, i know this is more a style issue but: why use a for loop when for all practical purposes you're using it as a while loop? :)