LibRaw 0.18.13 (update: was 0.18.3...0.18.12)

Submitted by lexa on

LibRaw 0.18.13 released and available on both download page and on Github repository.

This is bugfix release, changes are (compared to 0.18.2):

  • changed wrong fix for Canon D30 white balance
  • fixed possible stack overrun while reading zero-sized strings
  • fixed possible integer overflow
  • Secunia Advisory SA83507, credits Kasper Leigh Haabb, Secunia Research at Flexera
    • parse_qt: possible integer overflow
    • reject broken/crafted NOKIARAW files
    • Backported 0.19-patch to recover read position if TIFF/EXIF tag is too long
  • Secunia Advisory SA83050: possible infinite loop in parse_minolta()
  • Fixed stack overrun in kodak_radc_load_raw
  • restored static for utf2char() lost in previous bugfix
  • Fixed possible div by zero in EOS D30 WB data parse
  • packed_load_raw(): EOF check on each row
  • Exceptions was not caught in x3f_new_from_file resulting in x3f handle leak
  • CVE-2018-10529 fixed: out of bounds read in X3F parser
  • CVE-2018-10528 fixed: possible stack overrun in X3F parser
  • samsung_load_raw: possible buffer overrun
  • rollei_load_raw: possible buffer overrun
  • nikon_coolscan_load_raw: possible buffer overrun, possible NULL pointer
  • find_green: possible stack overrun
  • parse_exif: possible stack overrun
  • leaf_hdr_load_raw: check for image pointer for demosaiced raw
  • NOKIARAW parser: check image dimensions readed from file
  • quicktake_100_load_raw: check width/height limits
  • All legacy (RGB raw) image loaders checks for imgdata.image is not NULL
  • kodak_radc_load_raw: check image size before processing
  • legacy memory allocator: allocate max(widh,raw_width)*max(height,raw_height)
  • Fixed fuji_width handling if file is neither fuji nor DNG
  • Fixed xtrans interpolate for broken xtrans pattern
  • Fixed panasonic decoder
  • Fix for possible buffer overrun in kodak_65000 decoder
  • Fix for possible heap overrun in Canon makernotes parser
  • Fix for CVE-2017-13735
  • CVE-2017-14265: Additional check for X-Trans CFA pattern data
Please note: fixed bugs do not affect real from-camera files processing, you need to feed LibRaw by specially crafted files (e.g. run online service that accepts any file) to be affected by these problems.

Comments

compiler warning

Submitted by Matt Black (not verified) on

Hi, I'm getting the following warning from gcc-6.4.0 in libraw_cpp.cxx:

* /tmp/portage/portage/media-libs/libraw-0.18.5/work/LibRaw-0.18.5/src/libraw_cxx.cpp:1784:64: warning: iteration 6 invokes undefined behavior [-Waggressive-loop-optimizations]

CFLAGS="-O2 -march=native -ftree-vectorize -mprefer-avx128 -mvzeroupper -fstack-protector -ftree-loop-distribution -ftree-loop-distribute-patterns -pipe"

Code:

static inline void unpack28bytesto16x16ns(unsigned char *src, unsigned short *dest)
{
dest[0] = (src[3] << 6) | (src[2] >> 2);
dest[1] = ((src[2] & 0x3) << 12) | (src[1] << 4) | (src[0] >> 4);
dest[2] = (src[0] & 0xf) << 10 | (src[7] << 2) | (src[6] >> 6);
dest[3] = ((src[6] & 0x3f) << 8) | src[5];
dest[4] = (src[4] << 6) | (src[11] >> 2);
dest[5] = ((src[11] & 0x3) << 12) | (src[10] << 4) | (src[9] >> 4);
dest[6] = (src[9] & 0xf) << 10 | (src[8] << 2) | (src[15] >> 6);
dest[7] = ((src[15] & 0x3f) << 8) | src[14];
dest[8] = (src[13] << 6) | (src[12] >> 2);
dest[9] = ((src[12] & 0x3) << 12) | (src[19] << 4) | (src[18] >> 4);
dest[10] = (src[18] & 0xf) << 10 | (src[17] << 2) | (src[16] >> 6);
dest[11] = ((src[16] & 0x3f) << 8) | src[23]; ## line 1784
dest[12] = (src[22] << 6) | (src[21] >> 2);
dest[13] = ((src[21] & 0x3) << 12) | (src[20] << 4) | (src[27] >> 4);
dest[14] = (src[27] & 0xf) << 10 | (src[26] << 2) | (src[25] >> 6);
dest[15] = ((src[25] & 0x3f) << 8) | src[24];
}

gcc line numbering

Submitted by Matt Black (not verified) on

gcc preprocessor does not count blank and comment lines, and I gave the wrong code section. sorry.

I pulled the rest of the warnings from the build log:

/tmp/portage/portage/media-libs/libraw-0.18.5/work/LibRaw-0.18.5/src/libraw_cxx.cpp: In member function ‘virtual int LibRaw::open_datastream(LibRaw_abstract_datastream*)’:

/tmp/portage/portage/media-libs/libraw-0.18.5/work/LibRaw-0.18.5/src/libraw_cxx.cpp:1784:64: warning: iteration 6 invokes undefined behavior [-Waggressive-loop-optimizations]

imgdata.idata.xtrans[0]

 = imgdata.idata.xtrans_abs[0][c];
                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
 
 
/tmp/portage/portage/media-libs/libraw-0.18.5/work/LibRaw-0.18.5/src/libraw_cxx.cpp:1783:22: note: within this loop
 
     for(int c = 0; c < 36; c++)
                    ~~^~~~
 
So it appears the actual code section begins at line 1999:
 
 
    // XTrans Compressed?
    if (!imgdata.idata.dng_version && !strcasecmp(imgdata.idata.make, "Fujifilm") &&
        (load_raw == &LibRaw::unpacked_load_raw))
    {
      if (imgdata.sizes.raw_width * imgdata.sizes.raw_height * 2 != libraw_internal_data.unpacker_data.data_size)
      {
        if (imgdata.sizes.raw_width * imgdata.sizes.raw_height * 7 / 4 == libraw_internal_data.unpacker_data.data_size)
          load_raw = &LibRaw::fuji_14bit_load_raw;
        else
          parse_fuji_compressed_header();
      }
      if (imgdata.idata.filters == 9)
      {
        // Adjust top/left margins for X-Trans
        int newtm = imgdata.sizes.top_margin % 6 ? (imgdata.sizes.top_margin / 6 + 1) * 6 : imgdata.sizes.top_margin;
        int newlm = imgdata.sizes.left_margin % 6 ? (imgdata.sizes.left_margin / 6 + 1) * 6 : imgdata.sizes.left_margin;
        if (newtm != imgdata.sizes.top_margin || newlm != imgdata.sizes.left_margin)
        {
          imgdata.sizes.height -= (newtm - imgdata.sizes.top_margin);
          imgdata.sizes.top_margin = newtm;
          imgdata.sizes.width -= (newlm - imgdata.sizes.left_margin);
          imgdata.sizes.left_margin = newlm;
          for (int c1 = 0; c1 < 6; c1++)
            for (int c2 = 0; c2 < 6; c2++)
              imgdata.idata.xtrans[c1][c2] = imgdata.idata.xtrans_abs[c1][c2];
        }
      }
}
 
 
gcc optimizes the nested 6x6 loop to:   for(int c = 0; c < 36; c++)
 
on the 6th iteration of that loop the result of  imgdata.idata.xtrans[c] = imgdata.idata.xtrans_abs[c];  
is undefined.

what is wrong with

Submitted by lexa on

what is wrong with 

for(int c = 0; c < 36; c++)

loop if both xtrans and xtrans_abs are [6][6] arrays, so xtrans[0][35] is equal to xtrans[5][5]?

-- Alex Tutubalin @LibRaw LLC

the loop is ok

Submitted by Matt Black (not verified) on

That is a very common optimization and is not likely to be the issue. The problem is usually found in the expression itself, commonly either the right-side array element is undefined or the result overflows the variable.

On the 6th iteration of that loop the result of this expression is undefined:

imgdata.idata.xtrans

 = imgdata.idata.xtrans_abs[c];

C++ standard defines arrays

Submitted by lexa on

C++ standard defines arrays as dense arrays, no spacing or extra rows padding.

Also, because xtrans/_abs arrays are defined in LibRaw class definition, and it is possible that we compile two source files with different -O.. switches, array layout should not changed from -O0 (no warning) to -O2 (warning issued).
So, I think this is gcc problem: it should either show this warning with any -O.. setting, or do not show it with -O2

Anyway, this loop was already changed to to 6-wide loops in master branch with commit message 'to make gcc happy' :)

-- Alex Tutubalin @LibRaw LLC