HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Reading a .raw file while looking for .jpeg signatures

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
readingfilewhilesignaturesrawlookingforjpeg

Problem

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
#include
#include

#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 * n

Solution

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

Code Snippets

FILE* inptr = fopen(infile, "r");
FILE* inptr = fopen(infile, "rb");
char filename[7];

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

Context

StackExchange Code Review Q#140496, answer score: 4

Revisions (0)

No revisions yet.