Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVE-2017-9614 #167

Closed
mmuehlenhoff opened this issue Aug 12, 2017 · 6 comments
Closed

CVE-2017-9614 #167

mmuehlenhoff opened this issue Aug 12, 2017 · 6 comments

Comments

@mmuehlenhoff
Copy link

The follow was crash reported on the full-disclosure mailing list (It also includes a PoC) and was assigned CVE-2017-9614
http://seclists.org/fulldisclosure/2017/Jul/66

@dcommander
Copy link
Member

dcommander commented Aug 14, 2017

I am unable to reproduce using djpeg. Please demonstrate the failure using only libjpeg-turbo. I don't debug other people's code.

@dcommander
Copy link
Member

@mmuehlenhoff Please demonstrate how to reproduce this failure using only libjpeg-turbo, or I will be forced to assume that it is a downstream bug and close this tracker issue. Follow through on your bug reports if you want them fixed. Thanks.

@dcommander
Copy link
Member

This is a bug in stills2dv caused by an abuse of the libjpeg API. This patch fixes it and probably improves performance as well.

index 18bfff2..a4cf7f2 100644
--- a/s2d_jpg.c
+++ b/s2d_jpg.c
@@ -64,11 +64,10 @@ Image *readjpg(char *fn)
   FILE *in;
   int i,bytecount;
   Image *res;
-  unsigned long location=0;
   unsigned char *ptr;
   struct jpeg_decompress_struct cinfo={0};
   struct jpeg_error_mgr jerr;
-  JSAMPROW row_pointer[1];
+  JSAMPROW *row_pointer=NULL;
   //printf("openImage(%s);\n", fn);
   TRACE;
   if((in=fopen(fn, "rb"))==NULL)
@@ -105,7 +104,7 @@ Image *readjpg(char *fn)
       return NULL;
     }
   // printf("about to alloc a row\n");
-  if((row_pointer[0] = (unsigned char *)malloc( cinfo.output_width*cinfo.num_components ))==NULL)
+  if((row_pointer = (unsigned char **)malloc( sizeof(JSAMPROW) * cinfo.output_height ))==NULL)
     {
       fprintf(stderr, "Out of memory, could not allocate row pointer\n");
       free(res->data);
@@ -114,13 +113,13 @@ Image *readjpg(char *fn)
       return NULL;
     }
   // printf("about to scan\n");
+  for (i=0; i<cinfo.output_height; i++)
+    row_pointer[i]=&res->data[i * cinfo.output_width * cinfo.num_components];
   while( cinfo.output_scanline < res->height )
     {
-      jpeg_read_scanlines( &cinfo, row_pointer, 1 );
-      for( i=0; i<res->width*cinfo.num_components;i++) 
-       res->data[location++] = row_pointer[0][i];
+      jpeg_read_scanlines( &cinfo, &row_pointer[cinfo.output_scanline],
+                           res->height-cinfo.output_scanline );
     }
-  free(row_pointer[0]);
   // printf("checking num_components\n");
   if(cinfo.num_components==1)
     {
@@ -132,6 +131,7 @@ Image *readjpg(char *fn)
           jpeg_finish_decompress(&cinfo);
          free(res->data);
          free(res);
+         free(row_pointer);
          return NULL;
        }
       for (i=0;i<bytecount;i++)
@@ -142,10 +142,11 @@ Image *readjpg(char *fn)
       res->data=ptr;
     }
   // printf("About to fclose(in);\n");
-  fclose(in);
   res->fn=strdup(fn);
   // printf("About to finish_decompress;\n");
   jpeg_finish_decompress(&cinfo);
+  fclose(in);
+  free(row_pointer);
   // printf("About to return with res=%p and res->data=%p\n", res, res->data);
   return res;

In the future, please do not submit bug reports against libjpeg-turbo unless the bug can be reproduced with libjpeg-turbo alone. Otherwise there is no way to know if the bug is a downstream bug, as this one was. There are numerous examples of how to properly use the libjpeg API to decompress an image, including in our own source code (in particular, https://github.com/libjpeg-turbo/libjpeg-turbo/blob/master/example.c#L283-L407 and https://github.com/libjpeg-turbo/libjpeg-turbo/blob/master/turbojpeg.c#L1381-L1487).

The fact that a CVE was assigned to this without someone even bothering to check whether it was an application bug rather than a library bug is ridiculous. @mmuehlenhoff, I hope that you and other parties will immediately rescind that CVE and post an appropriate retraction, as it makes our project look bad when someone accuses us of a vulnerability that doesn't actually exist (in point of fact, this same issue was reproducible with any version of libjpeg as well, since the issue involved an abuse of the libjpeg API.)

@stefson
Copy link

stefson commented Oct 9, 2017

Hey there, I read about the CVE and ended up here - just to confirm, I understood that this issue is not caused by your code, right? If so, may I kindly ask what this patch is about which you have pasted here?

@dcommander
Copy link
Member

"This is a bug in stills2dv caused by an abuse of the libjpeg API. This patch fixes it and probably improves performance as well."

Sorry, I don't know how to be more clear than that.

@stefson
Copy link

stefson commented Oct 9, 2017

Ah, okay, I thought it was some kind of submodule of libjpeg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants