I am learning how to code and I have no experience with that at all. I've successful got to PSET4 and stuck on recover. I've read everything online about this problem and i found out that many people have similar code as I do and it works. Does not work for me whatsoever. Please have a look and give me a hint what did I do wrong and how to correct it. Here is everything about the pset4 recover i downloaded their card.raw from here card.raw
/** recovering JPEG files from a memory card
*
*/
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
typedef uint8_t BYTE;
int main(int argc, char* argv[])
{
// ensure proper usage
if (argc != 2)
{
fprintf(stderr,
"Usage: ./recover infile (the name of a forensic image from which to recover JPEGs)\n");
return 1;
}
// open input file (forensic image)
FILE* inptr = fopen(argv[1], "r");
if (inptr == NULL)
{
fprintf(stderr, "Could not open %s.\n", argv[1]);
return 2;
}
FILE* outptr = NULL;
// create a pointer array of 512 elements to store 512 bytes from the memory card
BYTE* buffer = malloc(sizeof(BYTE) * 512);
if (buffer == NULL)
{
return 3;
}
// count amount of jpeg files found
int jpeg = 0;
// string for a file name using sprintf
char filename[8] = { 0 };
// read memory card untill the end of file
while (fread(buffer, sizeof(BYTE) * 512, 1, inptr) != 0)
{
// check if jpeg is found
if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff
&& (buffer[3] >= 0xe0 || buffer[3] <= 0xef))
{
if (jpeg > 0)
{
fclose(outptr);
}
sprintf(filename, "%03d.JPEG", jpeg);
outptr = fopen(filename, "w");
jpeg++;
}
if (jpeg > 0)
{
fwrite(buffer, sizeof(BYTE) * 512, 1, outptr);
}
}
// free memory
free(buffer);
// close filename
fclose(outptr);
// close input file (forensic image)
fclose(inptr);
return 0;
}
The main problem is that you invoke undefined behavior because filename
is not enough big. sprintf()
need be 9 and 17 bytes with your code but you only has 8. So you have a buffer overflow.
Just change:
char filename[8] = { 0 };
to
char filename[17] = { 0 };
Because, you use an int
, this value is implemented defined but in many system has an int
with 32 bits. So the value possible are between -2^31
and 2^31 - 1
that make a maximum of 11 char
s (-2147483648). We add the number of char
s in ".JPEG", 5. We have 16 but you forget the null terminate byte of a c-string. So we are 17 maximum.
Modern compiler warning you: gcc version 7.1.1 20170516 (GCC):
In function ‘main’:
warning: ‘sprintf’ writing a terminating nul past the end of the destination [-Wformat-overflow ]
sprintf(filename, "%03d.JPEG", jpeg++);
^
note: ‘sprintf’ output between 9 and 17 bytes into a destination of size 8
sprintf(filename, "%03d.JPEG", jpeg++);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Plus, your typedef
is useless because a char
world be always a byte in C. More than that you don't need a byte but an octet so like char
, uint8_t
would be always an octet in C. So you don't need typedef
.
Again one thing, you allocate your buffer but it's useless because your buffer has a constant size. So just create an array is more simple.
#include <stdint.h>
#include <stdio.h>
int main(int argc, char *argv[]) {
if (argc != 2) {
fprintf(stderr, "Usage: ./recover infile (the name of a forensic image "
"from which to recover JPEGs)\n");
return 1;
}
FILE *inptr = fopen(argv[1], "r");
if (inptr == NULL) {
fprintf(stderr, "Could not open %s.\n", argv[1]);
return 2;
}
FILE *outptr = NULL;
uint8_t buffer[512];
size_t const buffer_size = sizeof buffer / sizeof *buffer;
size_t jpeg = 0;
while (fread(buffer, sizeof *buffer, buffer_size, inptr) == buffer_size) {
if (buffer[0] == 0xff && buffer[1] == 0xd8 && buffer[2] == 0xff &&
buffer[3] == 0xe0) {
if (outptr != NULL) {
fclose(outptr);
}
char filename[26];
sprintf(filename, "%03zu.JPEG", jpeg++);
outptr = fopen(filename, "w");
}
if (outptr != NULL) {
fwrite(buffer, sizeof *buffer, buffer_size, outptr);
}
}
if (outptr != NULL) {
fwrite(buffer, sizeof *buffer, buffer_size, outptr);
}
if (outptr != NULL) {
fclose(outptr);
}
fclose(inptr);
}
Note: This example is clearly not perfect, this will be better to make a true parser for jpeg file to have a better control flow. Here we suppose that all gonna be right.