[Pkg-tigervnc-devel] Bug#768369: Acknowledgement ([libjpeg62-turbo] [DOS] Stack smashing)
DRC
dcommander at users.sourceforge.net
Fri Nov 21 16:20:31 UTC 2014
Hi, guys. So what's still tripping me up on this is that I can't
reproduce it using only libjpeg-turbo. There is really no action I can
take until I am able to do that, because without the ability to
reproduce the issue irrespective of ImageMagick, I have no way of
knowing whether this is truly a bug in libjpeg-turbo or whether it is
caused by ImageMagick doing something wrong and producing undefined
behavior. It would be helpful if someone could point me to the specific
code in ImageMagick that is causing the failure. Assume I have no time
to spend on this, so me building ImageMagick from source is probably not
going to happen anytime soon. Our best hope of fixing this is if
someone can help me isolate it and write a simple test program to
reproduce it outside of ImageMagick.
libjpeg-turbo's Huffman code was studied very carefully in recent
months, in the process of fixing this issue:
https://sourceforge.net/p/libjpeg-turbo/bugs/64
Specifically, take a look at this comment:
https://sourceforge.net/p/libjpeg-turbo/bugs/64/?limit=10&page=1#3aa9
which includes a test program that was used to reproduce the overrun.
130 bytes (2-byte overrun) was the maximum I could reproduce, even using
absolute worst-case artificially-constructed circumstances that will
never happen in real life. Even then, the issue occurred only a few
times out of literally a billion iterations, and after increasing the
local buffer size by 8 bytes, the original submitter of the bug report
has had no issues since then (and his software apparently hits the
compressor 200 million times a day.) That's why I increased the local
buffer size by 8 bytes. I don't have a problem with increasing it more,
but I need to understand what's happening first.
One thing that is important to note is that the Huffman local buffer is
only ever used if the memory destination manager is used. Thus, I
modified jpegtran using mozjpeg's patches (attached) that allow it to
use the memory destination manager. This was an attempt to simulate
what I thought ImageMagick might be doing, but it unfortunately didn't
reproduce the bug when I ran 'jpegtran -rotate 270 003632r270.jpg
>junk.jpg'. Note also that I am including
002_detect-buffer-overrun-in-jchuff_c.patch from above, which should
print an error if the Huffman local buffer is overrun. At this point,
I'm stymied. If someone can point me to the place in the ImageMagick
code where this is happening, I can at least take a look at how they're
calling the transformer and see how it differs from how I'm calling the
transformer.
On 11/15/14 10:51 AM, Bernhard Übelacker wrote:
> Hello,
> DRC suggested to have a look at the newer upstream version.
>
> In jchuff.c the buffer in question is there really grown.
> But only by 8 bytes. [1]
>
> When increasing by 28 bytes the stack smashing and writing beyond the
> buffer goes away.
>
> The resulting image "looks" good. (Input file from the first post.)
>
> If this is the right solution or if the buffer can grow even more I
> cannot say.
>
> Kind regards,
> Bernhard
>
> [1] http://sourceforge.net/p/libjpeg-turbo/code/1367/ and
> http://sourceforge.net/p/libjpeg-turbo/code/1364/
>
>
>
>
> On Sat, 15 Nov 2014 16:56:20 +0100
> =?UTF-8?B?QmVybmhhcmQgw5xiZWxhY2tlcg==?= <bernhardu at vr-web.de> wrote:
>> Hello,
>> probably the attached patch could help in diagnose the issue.
>> It prints an error message and aborts, when the current buffer
>> pointer is advanced past the _buffer.
>>
>> In debugger it shows this happens a little before what roucaries bastien in message 47 wrote.
>> (Because he stopped at the stack protector overwritten,
>> this is _buffer[137] while its size is only 128.)
>>
>> Kind regards,
>> Bernhard
>>
>>
>>
>>
>> $ gdb --args convert -rotate 270 003632r270.jpg junk.jpg
>>
>> (gdb) run
>>
>> jchuff.c, line 591: written beyond end of _buffer, size=128, _buffer=0x0x7fffffff3e10, buffer=0x0x7fffffff3e91, pos=129
>>
>> Program received signal SIGABRT, Aborted.
>>
>> (gdb) bt
>> #0 0x00007ffff7067107 in __GI_raise (sig=sig at entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
>> #1 0x00007ffff70684e8 in __GI_abort () at abort.c:89
>> #2 0x00007ffff36d4268 in encode_one_block (actbl=0x646920, dctbl=<optimized out>, last_dc_val=<optimized out>, block=0x7ffff2cf9bb0, state=0x7fffffff3dd0) at jchuff.c:591
>>
>> (gdb) up
>> (gdb) up
>> #2 0x00007ffff36d4268 in encode_one_block (actbl=0x646920, dctbl=<optimized out>, last_dc_val=<optimized out>, block=0x7ffff2cf9bb0, state=0x7fffffff3dd0) at jchuff.c:591
>> 591 kloop(44);
>>
>>
>>
-------------- next part --------------
Index: jpegtran.c
===================================================================
--- jpegtran.c (revision 1418)
+++ jpegtran.c (working copy)
@@ -42,6 +42,7 @@
static char * outfilename; /* for -outfile switch */
static JCOPY_OPTION copyoption; /* -copy switch */
static jpeg_transform_info transformoption; /* image transformation options */
+#define INPUT_BUF_SIZE 4096
LOCAL(void)
@@ -378,6 +379,10 @@
* single file pointer for sequential input and output operation.
*/
FILE * fp;
+ unsigned char *inbuffer = NULL;
+ unsigned long insize = 0;
+ unsigned char *outbuffer = NULL;
+ unsigned long outsize = 0;
/* On Mac, fetch a command line. */
#ifdef USE_CCOMMAND
@@ -447,7 +452,26 @@
#endif
/* Specify data source for decompression */
- jpeg_stdio_src(&srcinfo, fp);
+ {
+ size_t nbytes;
+ do {
+ inbuffer = (unsigned char *)realloc(inbuffer, insize + INPUT_BUF_SIZE);
+ if (inbuffer == NULL) {
+ fprintf(stderr, "%s: memory allocation failure\n", progname);
+ exit(EXIT_FAILURE);
+ }
+ nbytes = JFREAD(fp, &inbuffer[insize], INPUT_BUF_SIZE);
+ if (nbytes < INPUT_BUF_SIZE && ferror(fp)) {
+ if (file_index < argc)
+ fprintf(stderr, "%s: can't read from %s\n", progname,
+ argv[file_index]);
+ else
+ fprintf(stderr, "%s: can't read from stdin\n", progname);
+ }
+ insize += (unsigned long)nbytes;
+ } while (nbytes == INPUT_BUF_SIZE);
+ jpeg_mem_src(&srcinfo, inbuffer, insize);
+ }
/* Enable saving of extra markers that we want to copy */
jcopy_markers_setup(&srcinfo, copyoption);
@@ -509,7 +533,7 @@
file_index = parse_switches(&dstinfo, argc, argv, 0, TRUE);
/* Specify data destination for compression */
- jpeg_stdio_dest(&dstinfo, fp);
+ jpeg_mem_dest(&dstinfo, &outbuffer, &outsize);
/* Start compressor (note no image data is actually written here) */
jpeg_write_coefficients(&dstinfo, dst_coef_arrays);
@@ -526,6 +550,27 @@
/* Finish compression and release memory */
jpeg_finish_compress(&dstinfo);
+
+ {
+ size_t nbytes;
+
+ unsigned char *buffer = outbuffer;
+ unsigned long size = outsize;
+ if (insize < size) {
+ size = insize;
+ buffer = inbuffer;
+ }
+
+ nbytes = JFWRITE(fp, buffer, size);
+ if (nbytes < size && ferror(fp)) {
+ if (file_index < argc)
+ fprintf(stderr, "%s: can't write to %s\n", progname,
+ argv[file_index]);
+ else
+ fprintf(stderr, "%s: can't write to stdout\n", progname);
+ }
+ }
+
jpeg_destroy_compress(&dstinfo);
(void) jpeg_finish_decompress(&srcinfo);
jpeg_destroy_decompress(&srcinfo);
More information about the Pkg-tigervnc-devel
mailing list