Reading a .raw file while looking for .jpeg signatures

Question

The code reads 512 bytes at a time and stores them in a buffer. When the code sees a .jpeg signature it will proceed by writing to a .jpeg file until another signature is found, thereafter the process repeats.

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

#define infile "card.raw"
#define BLOCK 512

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

// Declarations and prototype(s)
char * name();

FILE* outptr = NULL;
int increment = 0;

int main(void)
{

    // Open input file, plus error check
    FILE* inptr = fopen(infile, "r");
    if (inptr == NULL)
    {
        printf("Could not open %s.\n", infile);
        return 3;
    }


    // Allocate memory for one BLOCK and read from infile
    unsigned char * buffer = malloc(BLOCK);

    if (buffer == NULL)
        return 3;

    while (fread(buffer, BLOCK, 1, inptr) != 0) {

        /* Shall modify to operate on bitwise operations in the future. */

        // If the first four bytes of buffer is equal to a jpg file signature,
        // then copy all contents to a new jpg until another signature is found
        if (buffer[0] == 0xff &&
        buffer[1] == 0xd8 &&
        buffer[2] == 0xff &&
        buffer[3] >= 0xe0 && buffer[3] <= 0xef) {

            // If outptr does not yet refer to a file, close it
            if (outptr != NULL)
                fclose(outptr);

            // Open ###th output file, plus error-checking
            outptr = fopen(name(), "w");
            if (outptr == NULL) {
                fprintf(stderr, "Could not create outfile.\n");
                fclose(outptr);
                return 3;
            }

        }
        if (outptr != NULL)
            fwrite(buffer, BLOCK, 1, outptr);
    }
    // Check if the file was looped through correctly
    if ( !(feof(inptr)) )
        return 3;


    free(buffer);

    fclose(inptr);

    return 0;
}


// Name files incrementally starting from '000'
int filenum = 0;
char filename[7];

char * name()
{
    if (filenum < 10)
        sprintf(filename, "00%d.jpg", filenum);
    else
        sprintf(filename, "0%d.jpg", filenum);

    filenum++;
    return filename;
}

Answer 1

Binary mode

You should use binary mode to open your file instead of text mode. So change:

FILE* inptr = fopen(infile, "r");

to

FILE* inptr = fopen(infile, "rb");

Otherwise, your CR/LF characters may get converted when you don't want them to be.

Buffer overrun

Your filename buffer is too short:

char filename[7];

char * name()
{
    if (filenum < 10)
        sprintf(filename, "00%d.jpg", filenum);
    else
        sprintf(filename, "0%d.jpg", filenum);
    // ...

Here, your filenames will be something like "000.jpg" which require 7 characters plus a terminating null character, or 8 characters in total. Also, if you generate 100 or more files, you will need even more characters.

Pad string with leading zeros

There is a better way to generate filenames padded with leading zeros. Instead of:

if (filenum < 10)
    sprintf(filename, "00%d.jpg", filenum);
else
    sprintf(filename, "0%d.jpg", filenum);

you can do this:

sprintf(filename, "%03d.jpg", filenum);