Recover pset4 Problem with memory

Question

Hello I have written this code to recover the images for pset4 recover.

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

int main (int argc, char *argv[])
{
    // ensure proper usage
    if (argc != 2)
    {
        fprintf(stderr, "Usage: copy infile outfile\n");
        return 1;
    }

    // remember memory name
    char *mem = argv[1];

    // open memory card forensic for reading only
    FILE *memory = fopen ("card.raw", "r");
    if (memory == NULL)
    {
        fprintf(stderr, "Could not open %s.\n", mem);
        return 2;
    }

    // allocate memory for the blocks that are read from memory
    unsigned char* block = malloc(512*sizeof(char));
    char filename [4];
    int counter = 1;
    FILE* image = NULL;

    // read the memory card
    while (fread(block, 512, 1, memory) == 1)
    {
        // check for the start of a jpg
        if (block[0]  == 0xff && block[1] == 0xd8 && block[2] == 0xff && (block[3] & 0xe0) == 0xe0)
        {
            // close current image if one is open
            if (image != NULL)
            {
                fclose(image);
            }
            // open new image
            sprintf(filename, "%03d.jpg", counter);
            image = fopen(filename, "w");
            counter++;
        }

        // write bytes to image
        if (image != NULL)
        {
            fwrite(block, 512, 1, image);
        }

    }
    fclose(image);
    fclose(memory);
    free(block);
}

This code generates corrupt images. After debugging it I found that the first two bytes directed by the pointer block change after sprintf creates the string filename. I was wondering why is this so?

Thanks

So, Cliff B answered this question and really helped me understand what was worng with this program. But Now I have another question regarding this post and is the reason why this happens. I thought that by using malloc I would be allocating memory in the heap and the array filename is allocated in the stack. Why do the "extra characters" in filename overwrite what is supposedly allocated in the heap?

Answer 1

Well, you have nearly tracked down the culprit and almost completely answered the question yourself. You came sooo close to the solution on your own! This is good.

This is a somewhat common error for this pset. Look at the following two lines:

char filename [4];
    ...
        sprintf(filename, "%03d.jpg", counter);

The sprintf command will store the filename, using 8 chars - 7 for the file name plus 1 for the end of string marker \0. %03d will store 3 digits, but doesn't stop there. Those 3 chars are followed by ".jpg" and then \0. That's 8 chars. Unfortunately, only 4 chars were allocated to filename. The sprintf() call is still going to write 8 chars, and the extra 4 chars are going to overwrite whatever is physically stored next in memory. Guess what that happens to be!!! ;-)

Ask yourself, why were only 4 chars allocated to the filename variable? How many should be allocated?

If this answers your question, please click on the check mark to accept. Let's keep up on forum maintenance. ;-)