CODEWITHSUNDEEP
c
GeekRome
GeekRome

Reputation: 23

sprintf changes the content of an apparently unrelated array

I've spent hours pondering about this problem but I just can't seem to understand what's happening with my code (bit of disclaimer here: I just started learning C).

Background

I'm implementing a code which reads 512 bytes blocks of data from a file (till EOF) using fread() and if the first four bytes of buffer are 0xff, 0xd8, 0xff and anywhere between 0xe0 and 0xef, the program will write to another file till the next 512 bytes block contains the same four bytes identifier. Sample code is found below:

...

typedef uint8_t BYTE;
BYTE bytes[512];
while (fread(bytes, 512, 1, file) == 1)
{
 while (bytes[0] == 0xff && bytes[1] == 0xd8 && bytes[2] == 0xff && (bytes[3] & 0xf0)  == 0xe0)
  {
   for (int j = 0; j < 4; j++) // Location 1
   {
        printf("Byte is %d\n", bytes[j]);
   }

   sprintf(newfile, "%03i.jpg", i);

   for (int j = 0; j < 4; j++) // Location 2
   {
        printf("Byte is %d\n", bytes[j]);
   }

   .
   .
   .
  }
}

I added the for loops to debug my code and realised that Location 1 produces the correct first 4 bytes (i.e. 0xff, 0xd8, 0xff, between 0xe0 and 0xef), while location 2 produces incorrect first 4 bytes. Could anyone please enlighten me about why the location matters here?

Upvotes: 0

Views: 275

Answers (1)

Jabberwocky
Jabberwocky

Reputation: 50831

With char newfile[6];, newfile can hold a string of at most length 5 (you need one char more for the NUL terminator).

sprintf(newfile, "%03i.jpg", i); (with i=0) will result in this filename: 000.jpg which takes 8 characters including the NUL terminator. Therefore you are experiencing a buffer overflow which is undefined behaviour which in your case manifests ifself by overwriting the bytes array.

You should never use sprintf but snprintf instead:

snprintf(newfile, sizeof(newfile), "%03i.jpg", i);

In that case no buffer overflow can occur, even if the newfile buffer is too small.

Upvotes: 0

Related Questions