LibRaw 0.18.6 (update: was 0.18.3, 0.18.4, and 0.18.5)

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

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

  • 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 are 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

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

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

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

the loop is ok

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

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