Pset4, recover: Writing problem

Question

Just finished recover.c, it worked and passed the check, happiness! Next I started my work on design, cut and recombine so many things. Now, I guess, my algorithm is efficient, but something went wrong on writing:
1) 512 bytes blocks read - ok
2) start of jpg check - ok (checked with debugger and "ftell")
3) create new jpg when needed - ok (creates 50 jpg's)
4) write there proper information - FAIL (file format error)
5) end when needed - ok

Please help me find, why writing data is wrong.

PS: whats the point of allocating buffer on heap (byte* buffer = malloc(512 * sizeof(byte));) or on stack (byte buffer[512];)? Which way is better and why?

/**
 * recover.c
 *
 * Computer Science 50
 * Problem Set 4
 * 
 * Semyon Pilyukov
 * [email protected]
 *
 * Recovers JPEGs from a forensic image.
 */

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

#define BLOCK 512
typedef uint8_t byte;

int main(int argc, char* argv[])
{
    // Open the card.raw file
    FILE* card = fopen("card.raw", "r");
    if (card == NULL)
    {
        fclose(card);
        printf("Unable to open a file\n");
        return 1;
    }

    // start count files
    int counter = 0;

    // take memory for buffer
    byte buffer[BLOCK];

    // proper aaray for filenames
    char title[7];

    // declare file pointer
    FILE* img = NULL;

    int pos = 0;

    // start searching jpg files
    while ((fread(buffer, sizeof(byte), BLOCK, card) == BLOCK))
    {
        pos = ftell(card);
        printf("before file %i\n", pos);

        // if first file found
        if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] == 0xe0 || buffer[3] == 0xe1))
        {
            pos = ftell(card);
            printf(" first file %i\n", pos);

            // create first file:
            // proper name
            sprintf(title, "%03d.jpg", counter);

            // create-open file
            img = fopen(title, "w");
            if (img == NULL)
                {
                    fclose(img);
                    printf("Unable to create a file");
                    return 1;
                }

            counter++;

            // write first block
            if ((fwrite(buffer, sizeof(byte), BLOCK, img) != BLOCK))
                printf("block write error!\n");

            // continue writing and creating new files if needed
            while ((fread(buffer, sizeof(byte), BLOCK, card) == BLOCK))
            {
                pos = ftell(card);

                // if new file found
                if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] == 0xe0 || buffer[3] == 0xe1))
                {
                    // close previous file
                    fclose(img);

                    // create next file:
                    // proper name
                    sprintf(title, "%03d.jpg", counter);

                    // create-open file
                    img = fopen(title, "w");
                    if (img == NULL)
                    {
                        fclose(img);
                        printf("Unable to create a file");
                        return 1;
                    }

                    counter++;
                }

                // continue writing
                if ((fwrite(buffer, sizeof(byte), BLOCK, img) != BLOCK))
                        printf("block write error!\n");
            }
            // close opened img file
            fclose(img);
        }   
    }
    fclose(card);
    return 0;
}

Answer 1

The problem is that the code has a memory overwrite problem that is corrupting data while it's being processed. It's very subtle and easily missed.

char title[7];

Since title is only 7 chars long, it doesn't allow room for the end of string marker. When a file name is created, the end of string marker, \0, is being written beyond the end of the array. It looks like that is the first byte of the buffer, corrupting the file.

Easily fixed by changing the array size to 8.

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