C program that recovers lost JPEG files

My overall strategy is to load a block size (512bytes) into memory and check if that blocks first 4 bytes match a JPEGS first 4 bytes.

If it does then, I'll open a new out file and start writing the bytes into the out file. I'll repeat this for every "lost" JPEG.

You can assume that the "lost" JPEGs are stored consecutively.

```
#include
#include
#include
#include

int main( int argc, char *argv[] )
{
//check for proper usage
if ( argc != 2 )
{
fprintf(stderr, "Proper usage: ./recover file\n" );
return 1;
}

//Remember name of file
char *infile = argv[1];

//open infile
FILE *inptr = fopen( infile, "r");
if ( inptr == NULL )
{
fprintf(stderr, "Could not open %s, please try again\n", infile );
return 2;
}

//INIT block size and buffer
int blockSize = 512;
unsigned char buffer[blockSize];

//Storage for file name and file
char fileBuffer[8];
FILE *outptr = NULL;

//count how many images have been found
int imageCount = 0;

//iterate over the infile 512 bytes at a time
while ( fread( buffer, blockSize, 1, inptr ) == 1 )
{
//look for jpeg header
if ( buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff && (buffer[3] & 0xf0) == 0xe0 ) {

//close output file if its open
if( outptr != NULL)
{
fclose(outptr);
}

//create the new file name and open that file
sprintf( fileBuffer, "%03d.jpg", imageCount );

outptr = fopen( fileBuffer, "w" );
if ( outptr == NULL )
{
fprintf( stderr, "Could not create %s", fileBuffer );
return 3;
}

//INCREMENT Image count
imageCount++;
}

//write to out file if an image was found
if( outptr != NULL )
{
fwrite( buffer, blockSize, 1, outptr);
}

}

//close last photo
fclose(outptr);

//close input file
fclose(inptr);

//success
return 0

It looks like the choice of area to scan and iteration through the disk is outside the scope of this program, so we'll assume that this is a decent way of finding and recovering a jpeg file.

Implementation:

• It looks like your program is set up to scan/find multiple images within the given region. As it is currently written, it will output 512 byte blocks per file if a given 512 byte block happened to start with the JPEG header. For this to be more or less general purpose, you should scan the block for the jpeg header, and then output, potentially across multiple blocks, to the same output file until you reach the end of the JPEG (FF D9, although I don't know if that can be found as random data within the image - if it can, you must parse the size fields of the format).

• Going off of the Wikipedia article you can use more fields than FF D8 FF E0 (for instance, 4A 46 49 46 00 at a 4 byte offset after the initial magic. You also don't need to perform the (buffer[3] & 0xf0) operation - it looks like the whole E0 is part of the spec (although maybe some image readers don't care, but that's another story). Using a longer magic will reduce your number of false positives.

• Maybe a "resume on failure" functionality could be useful? An output of where you were when the failure happened, as well as an allowed optional argument that specifies where to start within the block (you'll have to strtoull it or whatever, and then fseek in the file).

Code:

• sprintf( fileBuffer, "%03d.jpg", imageCount ); can overflow the buffer. %03d guarantees that the name will be at least 3 characters (+4 from ".jpg" and +1 for the terminating '\0' for a total of 8). If your program finds more than 999 images, this sprintf will start corrupting memory. Use snprintf instead (which will truncate memory before corrupting if the correct arguments are passed - a good choice to pretty much always use it instead). In theory you could make your buffer size based on the maximum printable length of the imageCount, but for it to be truly portable you'd have to use uint32_t (or some fixed size) and can then base the buffer size on the printable length of UINT32_MAX.

• If you use snprintf with the current buffer size, your program will start mangling names (to "1000.jp") as soon as it hits 1000 images.

• You can put a "max check" in your loop so that you don't start overriding files at some point, although you'll probably never pass in input data that writes 4 mil images. E.g. if (imageCount == UINT32_MAX) (or whatever is appropriate for the type chosen).

• fwrite( buffer, blockSize, 1, outptr); can fail, treat/warn/error.

• fclose(outptr); can fail as well (in both spots), for the same reason as above. fwrite/fclose will buffer "under the hood" and call fflush at certain points to write out to disk. These fflush operations can fail at the whim of the filesystem.

• fclose(inptr); in theory this can return some failure code as well, but since it's an input file I'm not 100% sure it's possible at all.

StackExchange Code Review Q#157754, answer score: 3