this is my code:

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

int main(int argc, char *argv[])
    // Check usage
    if (argc != 2)
        return 1;

    // Open file
    FILE* file = fopen(argv[1], "r");
    if (!file)
        return 1;

    // Read first three bytes
    // unsigned char data[999999];
    // int size = fread(data, 1, 999999, file);

    int num = -1;

    while(1 == 1){
        unsigned char data[512];
        size_t size = fread(data, 1, 512, file);
        if(size < 512 / 4) break;
        if (data[0] == 0xff && data[1] == 0xd8 && data[2] == 0xff)
        if (num > -1)
            char filename[7];
            sprintf(filename, "%03i.jpg", num);
            FILE* jpeg = fopen(filename, "w");
            fwrite(&data, 1, 512, jpeg);

    // Close file
    return 0;

and this is what check50 says:

:) recover.c exists.
:) recover.c compiles.
:) handles lack of forensic image
:( recovers 000.jpg correctly
    recovered image does not match
:( recovers middle images correctly
    recovered image does not match
:( recovers 049.jpg correctly
    recovered image does not match

My code does work (if the images are supposed to be completely transparent) but check 50 says something is off :(. I really need some help and i don't want to use the code from other question(s) like this one that did slightly better than mine in check50, so please don't mark this as duplicate. Thanks for the help!


No, your code only partially works. It is creating 50 files, but they don't contain valid images. Also, if you run ls -al *.jpg, you'll see that every file is 512 bytes long.

After the first signature is found, the code opens a file and writes 512 bytes to that file. The real problem is that it is reopening the file in 'w' mode. This means that each time a 512 byte block is read, the existing file is deleted a new file is created, and the current 512 bytes is written out - but the previous data is lost because the file was deleted when the latest fopen is executed.

The logic is mostly sound, but the output file should only be opened once and only when a new signature is found. For all the data between signature blocks, the data blocks should simply be written to the already opened file.

Also, the signature check is only checking the first 3 bytes. It needs to check the 4th byte too.

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

  • i used the infinite loop the make the program not have to keep track and only stop when i say so for simplicity and efficiency. i will try my best to implement what you have said and if it works i will gladly press the check mark, but either way thanks for trying! :) – person the human Jun 15 '20 at 21:51
  • Update: I can't see why my code would ignore all the data in the image besides the canvas. what i imagine my code does is (in pseudo code): 1: read next 512 bytes of data. 2: if they contain the jpeg header, then increase num (which starts at -1). 3: if num > -1, copy all newly read data into the jpeg that i am currently on. this should repeat untill i am less than 512 bytes away from EOF (which i guess could affect final image but i will fix it later). what am i missing and how could i implement what you desctibed in your answer? – person the human Jun 15 '20 at 22:02
  • Jeez. I must've been half asleep. I'll edit my answer to correct a misinterpretation that I made. But the problem is still that the files only contain 512 bytes. – Cliff B Jun 16 '20 at 4:32
  • oh! i thought w mode added new data. I guess that's why there's an 'a' mode too, because i used to think they did the same thing. – person the human Jun 16 '20 at 22:06

The following code cause memory corruptions.

            char filename[7];  // this is too small
            sprintf(filename, "%03i.jpg", num);  // this takes 8 bytes. 
  • Actually, that line was not the problem. – person the human Jun 16 '20 at 22:54

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .