Im busy with problem set 4(recover) and the image says Invalid or unsupported format. The sizes of the images look legit

Question

I am not sure why my images are unsupported. Everything looks as if it works until you try to open the images. Even the image sizes look correct.(meaning they aren't 0bytes or all the same size or 512 bytes) Why are the images unsupported?

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

int main(int argc, char *argv[])
{
    if (argc != 2)
    {
        printf("Usage: ./recover forensic_image\n");
        return 1;
    }

    FILE *raw = fopen(argv[1], "r");
    if (raw == NULL)
    {
        printf("Could not open file.\n");
        return 2;
    }
    
    int buffer[512];
    int JPEG_num = 0;
    FILE *img[50];
    char filename[9];

    while(true)
    {
        for(int i = 0; i < 512; i++)
        {
            fread(&buffer[i], 1, 1, raw);
        }
        if(buffer[0] == 0xff && buffer[1] == 0xd8 && (buffer[3] >= 0xe0 && buffer[3] <= 0xef))
        {
            if(JPEG_num == 0)
            {
                sprintf(filename, "%03i.jpg", 0);
                img[0] = fopen(filename, "w");
                fwrite(&buffer, 1, 512, img[0]);
                JPEG_num++;
            }
            else
            {
                fclose(img[JPEG_num - 1]);
                sprintf(filename, "%03i.jpg", JPEG_num);
                img[JPEG_num] = fopen(filename, "w");
                fwrite(&buffer, 1, 512, img[JPEG_num]);
                JPEG_num++;
            }
        }
        else
        {
            if(JPEG_num != 0)
            {
                fwrite(&buffer, 1, 512, img[JPEG_num - 1]);
            }
        }
        if(JPEG_num >= 50)
        {
            return false;
        }
    }
    fclose(img[JPEG_num - 1]);
    return 0;
}

Answer 1

It's a subtle but critical error. The problem lies here;

int buffer[512];
...
for(int i = 0; i < 512; i++)
    {
        fread(&buffer[i], 1, 1, raw);
    }

Here's what's happening. The fread() call reads exactly one byte and stores it in one element of the buffer[] array. But, each element in the array is 4 bytes long! The remaining bytes are filled with 0x00. That means that the code is storing 000000 in hex between every byte from the image! Massive file corruption, so you get the error message.

The code needs to store each byte in an element that's one byte long, not 4. Also, certain data types will alter how data is treated when it is stored. For instance, int will treat any data that looks like a negative numebr differently. (See "twos-complement".)

The fix is simple. Instead of using the int data type for buffer, use one that is one byte long and doesn't look at negative nummbers. Hint: research the uint8_t data type.

Next, take a careful look at how the while loop ends and what happens. The loop terminates when the return statement executes. The problem here is that the return statement immediately terminates the program. So, none of the code that follows is executed. That means that any files that are open will still be open when the program terminates. (OK, technically, they'll close by the system cleanup, but it's still a bad practice.)

Instead, maybe the while loop should end when the input file hits EOF??? Have you considered how you could combine the while loop with the fread call? Hint: what are the return values from fread()? ;-)

Programming tip: The for loop here does 512 separate reads, which is terribly inefficient, especially when reading from a spinning disk! You can do a single read of 512 bytes and insert it into the buffer in one command.

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