Infinite loop in jpeg_start method

Submitted by Soruk on

Hello,

We detected when using FreeImage an therfore LibRaw that for some JPEG files the jpeg_start method (from dcraw.common.cpp) the do while loop does not exists (there is no tag != 0xffda in the given JPEG file, but the image can be easily viewed on Windows using Paint for example).

Here is the fixed version fo the code (we added test for the EOF):

int CLASS ljpeg_start (struct jhead *jh, int info_only)
{
  int c, f, tag, len;
  uchar data[0x10000];
  const uchar *dp;
 
  memset (jh, 0, sizeof *jh);
  jh->restart = INT_MAX;
  fread (data, 2, 1, ifp);
  if (data[1] != 0xd8) return 0;
  do {
    fread (data, 2, 2, ifp);
	f = getc(ifp);//for EOF testing
	tag =  data[0] << 8 | data[1];
    len = (data[2] << 8 | data[3]) - 2;
 
// printf ("\n*** ljpeg_start pos= %llx tag= %x, len= %d", ftell(ifp)-4, tag, len);
 
    if (tag <= 0xff00) return 0;
    fread (data, 1, len, ifp);
    switch (tag) {
      case 0xffc3:        // start of frame; lossless, Huffman
	jh->sraw = ((data[7] >> 4) * (data[7] & 15) - 1) & 3;
//	printf ("\n*** %x: startraw= %d", tag, jh->sraw);
      case 0xffc0:        // start of frame; baseline jpeg
	jh->bits = data[0];
	jh->high = data[1] << 8 | data[2];
	jh->wide = data[3] << 8 | data[4];
	jh->clrs = data[5] + jh->sraw;
 
if (!strcmp(model, "EOS 5DS"))
{
  jh->wide = data[1] << 8 | data[2];
	jh->high = data[3] << 8 | data[4];
}
//	printf ("\n*** %x: bits= %d; high= %d; wide= %d; clrs= %d",
//	  tag, jh->bits, jh->high, jh->wide, jh->clrs);
 
	if (len == 9 && !dng_version) getc(ifp);
	break;
      case 0xffc4:          // define Huffman tables
	if (info_only) break;
	for (dp = data; dp < data+len && (c = *dp++) < 4; )
	  jh->free[geshifilter-c] = jh-&gt;huff[c] = make_decoder_ref (&amp;dp);&#10;	break;&#10;      case 0xffda:          // start of scan&#10;	jh-&gt;psv = data[1+data[0]*2];&#10;	jh-&gt;bits -= data[3+data[0]*2] &amp; 15;&#10;	break;&#10;      case 0xffdd:          // define restart interval&#10;	jh-&gt;restart = data[0] &lt;&lt; 8 | data[1];&#10;&#10;			break;&#10;		}&#10;	} while (f != EOF &amp;&amp; tag != 0xffda);&#10;&#10;// printf (&quot;\n&quot;);&#10;&#10;  if (info_only) return 1;&#10;  if (jh-&gt;clrs &gt; 6 || !jh-&gt;huff[0]) return 0;&#10;  FORC(5) if (!jh-&gt;huff[c+1]) jh-&gt;huff[c+1] = jh-&gt;huff[c];&#10;  if (jh-&gt;sraw) {&#10;    FORC(4)        jh-&gt;huff[2+c] = jh-&gt;huff[1];&#10;    FORC(jh-&gt;sraw) jh-&gt;huff[1+c] = jh-&gt;huff[0];&#10;  }&#10;  jh-&gt;row = (ushort *) calloc (jh-&gt;wide*jh-&gt;clrs, 4);&#10;  merror (jh-&gt;row, &quot;ljpeg_start()&quot;);&#10;  return zero_after_ff = 1;&#10;}&#10;

[/geshifilter-c]

Forums: 

Actual LibRaw already

Submitted by lexa on

Actual LibRaw already contains this check and additional loop count check

  memset (jh, 0, sizeof *jh);
  jh->restart = INT_MAX;
  fread (data, 2, 1, ifp);
  if (data[1] != 0xd8) return 0;
  do {
this line ==>     if(feof(ifp)) return 0;
and this line ==>    if(cnt++ > 1024) return 0; // 1024 tags limit

-- Alex Tutubalin @LibRaw LLC

Also, your patch will read

Submitted by lexa on

Also, your patch will read additional byte via f = getc(ifp);
I do not see any attempt to add this byte back to the input stream. Most likely, your code does not decode correct data right.

-- Alex Tutubalin @LibRaw LLC

Ok.

Submitted by Soruk on

Ok.
The FreeImage source code contains the old version without the fix (0.17.a1). I will include only the fix in our code.
Thanks.