#66 Bug in format_message() jerror.c

[karuna murti]

I post this on jpegoptim tracker which build on top of libjpeg-turbo
https://github.com/tjko/jpegoptim/issues/15

jpegoptim v1.4.0  x86_64-unknown-linux-gnu
Copyright (c) 1996-2014  Timo Kokkonen.

libjpeg version: 8d  15-Jan-2012
Copyright (C) 1991-2012 Thomas G. Lane, Guido Vollbeding
Copyright (C) 1999-2006 MIYASAKA Masaru
Copyright (C) 2009 Pierre Ossman for Cendio AB
Copyright (C) 2009-2014 D. R. Commander
Copyright (C) 2
*** stack smashing detected ***: jpegoptim terminated
Segmentation fault (core dumped)

---

ldd /usr/bin/jpegoptim 
    linux-vdso.so.1 (0x00007fff69ffe000)
    libm.so.6 => /usr/lib/libm.so.6 (0x00007fb838d46000)
    libjpeg.so.8 => /usr/lib/libjpeg.so.8 (0x00007fb838af1000)
    libc.so.6 => /usr/lib/libc.so.6 (0x00007fb838743000)
    /lib64/ld-linux-x86-64.so.2 (0x00007fb83904a000)

---

checking build system type... x86_64-unknown-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables... 
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking for a BSD-compatible install... /usr/bin/install -c
checking whether make sets $(MAKE)... yes
checking for jpeg_read_header in -ljpeg... yes
checking for round in -lm... yes
checking how to run the C preprocessor... gcc -E
checking for grep that handles long lines and -e... /usr/bin/grep
checking for egrep... /usr/bin/grep -E
checking for ANSI C header files... yes
checking for sys/types.h... yes
checking for sys/stat.h... yes
checking for stdlib.h... yes
checking for string.h... yes
checking for memory.h... yes
checking for strings.h... yes
checking for inttypes.h... yes
checking for stdint.h... yes
checking for unistd.h... yes
checking for unistd.h... (cached) yes
checking getopt.h usability... yes
checking getopt.h presence... yes
checking for getopt.h... yes
checking for string.h... (cached) yes
checking libgen.h usability... yes
checking libgen.h presence... yes
checking for libgen.h... yes
checking math.h usability... yes
checking math.h presence... yes
checking for math.h... yes
checking jpeglib.h usability... yes
checking jpeglib.h presence... yes
checking for jpeglib.h... yes
checking size of long... 8
checking size of int... 4
checking for getopt_long... yes
checking for mkstemps... yes
checking for broken jmorecfg.h (METHODDEF)... no
configure: creating ./config.status
config.status: creating Makefile
config.status: creating config.h
gcc  -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2 -DHAVE_CONFIG_H -D_FORTIFY_SOURCE=2  -c -o jpegoptim.o jpegoptim.c
gcc  -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2 -DHAVE_CONFIG_H -D_FORTIFY_SOURCE=2  -c -o jpegdest.o jpegdest.c
gcc  -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2 -DHAVE_CONFIG_H -D_FORTIFY_SOURCE=2  -c -o misc.o misc.c
gcc  -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong --param=ssp-buffer-size=4 -D_FORTIFY_SOURCE=2 -DHAVE_CONFIG_H -o jpegoptim jpegoptim.o jpegdest.o misc.o  -Wl,-O1,--sort-common,--as-needed,-z,relro -lm -ljpeg  
for i in jpegoptim ; do [ -x $i ] && strip $i ; done

---

core/zlib 1.2.8-3 [installed]
multilib/lib32-zlib 1.2.8-1 [installed]
extra/libjpeg-turbo 1.3.1-1 [installed]
multilib/lib32-libjpeg-turbo 1.3.1-1 [installed]

---

$ gdb ./jpegoptim 
GNU gdb (GDB) 7.7.1
Copyright (C) 2014 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ./jpegoptim...done.
(gdb) run --version
Starting program: /home/tom/arch-packages/jpegoptim/src/jpegoptim-RELEASE.1.4.0/jpegoptim --version
warning: Could not load shared library symbols for linux-vdso.so.1.
Do you need "set solib-search-path" or "set sysroot"?
jpegoptim v1.4.0  x86_64-unknown-linux-gnu
Copyright (c) 1996-2014  Timo Kokkonen.

libjpeg version: 8d  15-Jan-2012
Copyright (C) 1991-2012 Thomas G. Lane, Guido Vollbeding
Copyright (C) 1999-2006 MIYASAKA Masaru
Copyright (C) 2009 Pierre Ossman for Cendio AB
Copyright (C) 2009-2014 D. R. Commander
Copyright (C) 2
*** stack smashing detected ***: /home/tom/arch-packages/jpegoptim/src/jpegoptim-RELEASE.1.4.0/jpegoptim terminated

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff72ce688 in ?? () from /usr/lib/libgcc_s.so.1
(gdb) where
#0  0x00007ffff72ce688 in ?? () from /usr/lib/libgcc_s.so.1
#1  0x00007ffff72cf6f8 in _Unwind_Backtrace () from /usr/lib/libgcc_s.so.1
#2  0x00007ffff75cb786 in backtrace () from /usr/lib/libc.so.6
#3  0x00007ffff74f4a44 in backtrace_and_maps () from /usr/lib/libc.so.6
#4  0x00007ffff7548f8e in __libc_message () from /usr/lib/libc.so.6
#5  0x00007ffff75cee57 in __fortify_fail () from /usr/lib/libc.so.6
#6  0x00007ffff75cee20 in __stack_chk_fail () from /usr/lib/libc.so.6
#7  0x0000000000401dd5 in print_version () at jpegoptim.c:202
#8  0x6f697461726f7072 in ?? ()
#9  0x726f2f646e61206e in ?? ()
#10 0x6275732073746920 in ?? ()
#11 0x2879726169646973 in ?? ()
#12 0x000000297365692d in ?? ()
#13 0x0000000000000000 in ?? ()
(gdb) frame 7
#7  0x0000000000401dd5 in print_version () at jpegoptim.c:202
202 }
(gdb) list
197   COPY_JPEG_ERRSTR(&cinfo,jmsg_buf);
198   printf("\nlibjpeg version: %s\n",jmsg_buf);
199   cinfo.err->msg_code=JMSG_COPYRIGHT;
200   COPY_JPEG_ERRSTR(&cinfo,jmsg_buf);
201   printf("%s\n",jmsg_buf);
202 }
203 
204 
205 void own_signal_handler(int a)
206 {
(gdb)

Here's my compile flags:

$ make
gcc  -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong --param=ssp-buffer-size=4 -g -O0 -DHAVE_CONFIG_H   -c -o jpegoptim.o jpegoptim.c
gcc  -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong --param=ssp-buffer-size=4 -g -O0 -DHAVE_CONFIG_H   -c -o jpegdest.o jpegdest.c
gcc  -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong --param=ssp-buffer-size=4 -g -O0 -DHAVE_CONFIG_H   -c -o misc.o misc.c
gcc  -march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong --param=ssp-buffer-size=4 -g -O0 -DHAVE_CONFIG_H -o jpegoptim jpegoptim.o jpegdest.o misc.o   -lm -ljpeg


Edit: Here's a bt full from GDB:

(gdb) bt full
#0  0x00007ffff72ce688 in ?? () from /usr/lib/libgcc_s.so.1
No symbol table info available.
#1  0x00007ffff72cf6f8 in _Unwind_Backtrace () from /usr/lib/libgcc_s.so.1
No symbol table info available.
#2  0x00007ffff75cb786 in backtrace () from /usr/lib/libc.so.6
No symbol table info available.
#3  0x00007ffff74f4a44 in backtrace_and_maps () from /usr/lib/libc.so.6
No symbol table info available.
#4  0x00007ffff7548f8e in __libc_message () from /usr/lib/libc.so.6
No symbol table info available.
#5  0x00007ffff75cee57 in __fortify_fail () from /usr/lib/libc.so.6
No symbol table info available.
#6  0x00007ffff75cee20 in __stack_chk_fail () from /usr/lib/libc.so.6
No symbol table info available.
#7  0x0000000000401dd5 in print_version () at jpegoptim.c:202
        jcerr = {error_exit = 0x7ffff78a71b0, emit_message = 0x7ffff78a6fa0, 
          output_message = 0x7ffff78a7140, format_message = 0x7ffff78a7010, 
          reset_error_mgr = 0x7ffff78a6ff0, msg_code = 75, msg_parm = {i = {0, 
              0, 0, 0, 0, 0, 0, 0}, s = '\000' <repeats 79 times>}, 
          trace_level = 0, num_warnings = 0, 
          jpeg_message_table = 0x7ffff7ac68a0 <jpeg_std_message_table>, 
          last_jpeg_message = 126, addon_message_table = 0x0, 
          first_addon_message = 0, last_addon_message = 0}
        cinfo = {err = 0x7fffffff9670, mem = 0x0, progress = 0x0, 
          client_data = 0x0, is_decompressor = 0, global_state = 0, 
          dest = 0x0, image_width = 0, image_height = 0, 
          input_components = -136425708, in_color_space = 32767, 
          input_gamma = 0, scale_num = 4149051052, scale_denom = 32767, 
          jpeg_width = 4287062190, jpeg_height = 0, data_precision = 0, 
          num_components = 0, jpeg_color_space = 4160483072, 
          comp_info = 0x7ffff7de4f14 <check_match+228>, quant_tbl_ptrs = {0x0, 
            0x7ffff78837c4, 0x3e758475, 0x7ffff7de4f14 <check_match+228>}, 
          q_scale_factor = {-134484224, 32767, -145915728, 32767}, 
          dc_huff_tbl_ptrs = {0x27d5ae07, 0x0, 0x7ffff7fbef00, 
            0x7ffff7de566f <do_lookup_x+1727>}, ac_huff_tbl_ptrs = {
            0x7fffffff9800, 0x9f56b8, 0x7fff00000007, 0x7fffffff9920}, 
          arith_dc_L = " \230\377\377\377\177\000\000\020\231\377\377\377\177\000", arith_dc_U = "\320\344\373\367\377\177\000\000PK\210\367\377\177\000", 
          arith_ac_K = "\000\000\000\000\000\000\000\000`\320\373\367\377\177\000", num_scans = -134485600, scan_info = 0x400d4b, raw_data_in = -145859208, 
          arith_code = 32767, optimize_coding = 4195528, CCIR601_sampling = 0, 
          do_fancy_downsampling = 0, smoothing_factor = 1, 
          dct_method = (JDCT_IFAST | unknown: 1424), restart_interval = 1, 
          restart_in_rows = -134486832, write_JFIF_header = 32767, 
          JFIF_major_version = 136 '\210', JFIF_minor_version = 228 '\344', 
          density_unit = 255 '\377', X_density = 32767, Y_density = 0, 
          write_Adobe_marker = -26192, next_scanline = 32767, 
          progressive_mode = -134492064, max_h_samp_factor = 32767, 
          max_v_samp_factor = -134225616, min_DCT_h_scaled_size = 32767, 
          min_DCT_v_scaled_size = -26152, total_iMCU_rows = 32767, 
          comps_in_scan = 0, cur_comp_info = {
            0x7ffff7de5a6f <_dl_lookup_symbol_x+335>, 0x3, 0x7ffff7fbd060, 
            0x1}, MCUs_per_row = 0, MCU_rows_in_scan = 0, blocks_in_MCU = 1, 
          MCU_membership = {0, -134225616, 32767, -145881432, 1, 0, 0, 
            -134224760, 32767, -26336}, Ss = 32767, Se = -1, Ah = 1, 
          Al = -145707498, block_size = 32767, natural_order = 0x27d5ae07, 
          lim_Se = -142093716, master = 0x7fffffff9a00, 
          main = 0x401ac0 <_start>, prep = 0x7fffffffe5e0, coef = 0x2, 
          marker = 0x7ffff7880200 <getopt_data>, 
          cconvert = 0x7ffff75a6a73 <_getopt_internal_r+2099>, 
          downsample = 0xa8, fdct = 0x7fffffffe935, entropy = 0x8, 
          script_space = 0x1, script_space_size = 2}
        jmsg_buf = "Copyright (C) 1991-2012 Thomas G. Lane, Guido Vollbeding\nCopyright (C) 1999-2006 MIYASAKA Masaru\nCopyright (C) 2009 Pierre Ossman for Cendio AB\nCopyright (C) 2009-2014 D. R. Commander\nCopyright (C) 2"
#8  0x6f697461726f7072 in ?? ()
No symbol table info available.
#9  0x726f2f646e61206e in ?? ()

This is the jpegoptim maintainer reply:

Looks like this is a bug in libjpeg-turbo. Have you tested with (original) libjpeg?

It would seem like there is bug in format_message() function found in jerror.c, it looks like that function caused buffer overflow (that smashed stack).

Documenation for this function (in jerror.c) seem to imply is safe to call with buffer of JMSG_LENGTH_MAX length:

/*

• Format a message string for the most recent JPEG error or message.
• The message is stored into buffer, which should be at least JMSG_LENGTH_MAX
• characters. Note that no '\n' character is added to the string.
• Few applications should need to override this method.
  */

METHODDEF(void)
format_message (j_common_ptr cinfo, char * buffer)
However, the code in that function doesn't do any checking if it ends up going over that limit....

What triggers the buffer overflow seems to originate from jerror.h, that defines a enum list of message (codes). One of the codes (JMSG_COPYRIGHT) seems to be now too long. This code/message is currently 254 bytes long, while JMSG_MAX_LENGTH is set to 200.

It seems library expects all its own message to be shorter than JMSG_MAX_LENGTH, and that seems to lead buffer overflow in format_message() function (in jerror.c) when JMSG_COPYRIGHT code/message is outputted using the library's own methods.

This is pretty clearly bug in libjpeg-turbo, and should be fixed there, but I see if I can add workaround to avoid triggering this bug...

[DRC]

NOTE: In the future, if the bug is well understood, please refrain from posting pages of unnecessary debug output.

I can verify that this is reproducible with the following code:

#include <stdio.h>
#include <stdlib.h>
#include "jpeglib.h"
#include "jerror.h"

int main(void)
{
    struct jpeg_compress_struct cinfo;
    struct jpeg_error_mgr jerr;
    cinfo.err=jpeg_std_error(&jerr);
    jpeg_create_compress(&cinfo);
    cinfo.err->msg_code=JMSG_COPYRIGHT;
    cinfo.err->output_message((j_common_ptr)&cinfo);
}

It is easy enough to fix by changing JMSG_LENGTH_MAX to 400, but this really highlights a problem in the API, which is that format_message() should really have an additional parameter for the buffer length. I'm not going to open that can of worms, though, since it would break backward API and ABI compatibility.

Fortunately, since we're increasing the buffer internally, if a program that was compiled against a previous version of libjpeg-turbo still assumes that it should create a buffer 200 characters long, it will still work.

If you concur that changing JMSG_LENGTH_MAX is sufficient to fix this, then I'll check in that patch.

[Timo Kokkonen]

But since JMSG_LENGTH_MAX is technically part of the published API from IJG's libjpeg, changing this would break compatibility with libjpeg.

If program was compiled against libjpeg, there is still risk of buffer overflow happening in format_message(), if that program was at runtime linked against libjpeg-turbo...

Instead of changing JMSG_LENGTH_MAX, wouldn't it be better to fix format_message() function? So that it never writes more than JMSG_LENGTH_MAX characters to the output buffer [replace sprintf() calls with snprintf()...]

...or just shorten JMSG_COPYRIGHT, so that it doesn't exceed 200 bytes...

[DRC]

It wouldn't matter if the program was compiled against libjpeg and then used libjpeg-turbo at run time (because the app would assume the smaller buffer length and never exceed libjpeg-turbo's. It would matter if the app is compiled against libjpeg-turbo and assumes the larger buffer size, then tries to use libjpeg at run time. I don't think that's a very high-probability occurrence, but regardless, I agree that keeping the buffer length the same is best. The thing is, though, nowhere does it say that the buffer passed to format_message() has to be limited to JMSG_LENGTH_MAX. At least that was my recollection (not in front of the code right now.) But I think I can just provide a shorter message for JMSG_COPYRIGHT and display the longer one in cjpeg/djpeg.

[Timo Kokkonen]

If JMSG_LENGTH_MAX were to be increased, it could lead too short buffer being passed to format_message() function.
As some apps might be passing JMSG_LENGTH_MAX length buffer to format_messsage() function, so if this constant was smaller during compile time than what it is on the library that the program gets linked against at run-time, this could trigger buffer overflow.

Something like:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "jpeglib.h"
#include "jerror.h"

int main(void)
{
    char buf[JMSG_LENGTH_MAX];
    struct jpeg_compress_struct cinfo;
    struct jpeg_error_mgr jerr;
    cinfo.err=jpeg_std_error(&jerr);
    jpeg_create_compress(&cinfo);
    cinfo.err->msg_code=JMSG_COPYRIGHT;
    cinfo.err->format_message((j_common_ptr)&cinfo,buf);
    printf("'%s'\n",buf);
}

(I agree that this is likely very rare, but could happen...)

Above would have 200 byte buf when compiled against libjpeg, and 400 byte buf if compiled against libjpeg-turbo (with larger JMSG_LENGTH_MAX). So if compiled against libjpeg, it would run fine (against libjpeg), but crash when at run-time linked against libjpeg-turbo (that was compiled with larger JMSG_LENGTH_MAX):

~~~~~

./bug

'Copyright (C) 1991-2012 Thomas G. Lane, Guido Vollbeding
Copyright (C) 1999-2006 MIYASAKA Masaru
Copyright (C) 2009 Pierre Ossman for Cendio AB
Copyright (C) 2009-2014 D. R. Commander
Copyright (C) 2009-2011 Nokia Corporation and/or its subsidiary(-ies)'
*** stack smashing detected ***: ./bug terminated
Segmentation fault (core dumped)

~~~~~~

[DRC]

Would it be sufficient to just change the buffer size in output_message()?

The format_message() function requires that the buffer be "at least" JMSG_LENGTH_MAX characters, but it is OK for the buffer to be larger. Applications will never be able to directly access the JCOPYRIGHT string, so unless I miss my guess, the only way this string will ever overrun a buffer is if output_message() is called, and since the size of that buffer is internal, increasing it should not cause any compatibility issues.

[DRC]

Never mind. Upon further examination, I see that that would cause problems if format_message() was called directly by the application. I just included a shorter version of the copyright string for use with JMSG_COPYRIGHT (cjpeg, djpeg, etc. still display the longer version.) The fix has been checked into trunk, branches/1.3.x, and branches/1.2.x. Let me know if you encounter any further problems.