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);

• Sumurai8: I am not an expert in C, but does the code under // Open ###th output file, plus error-checking leak the memory of buffer?
• JS1: @Sumurai8 Yes it could if the function returns early. Also, in some cases, there is no fclose(outptr); before the function returns.