[Pkg-kde-extras] exiv2 stretch update (CVE-2018-16336)

Roberto C. Sánchez roberto at debian.org
Sun Oct 21 04:10:17 BST 2018


Hi all,

I prepared an update of exiv2 for jessie.  The patches I prepared
applied to the stretch version with only one minor change required.

The main change is the patch for CVE-2018-16336.  However, I also
included a tweak to the patch for CVE-2018-10958/CVE-2018-10999 based on
feedback I received approximately one month after I uploaded the last
security update for exiv2:

https://github.com/Exiv2/exiv2/issues/302#issuecomment-408640903

I have attached a debdiff from version 0.25-3.1+deb9u1 to
0.25-3.1+deb9u2 for your review and the actual packages are available
here:

https://people.debian.org/~roberto/

If the package and proposed changes look good, please let me know and I
can sign and upload the packages and someone on the security team can
publish the DSA.

Regards,

-Roberto

-- 
Roberto C. Sánchez
-------------- next part --------------
diff --git a/debian/changelog b/debian/changelog
index 0414fd3..01c9229 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,13 @@
+exiv2 (0.25-3.1+deb9u2) stretch-security; urgency=high
+
+  * Non-maintainer upload by the Security Team.
+  * Minor adjustment to the patch for CVE-2018-10958 and CVE-2018-10999.  The
+    initial patch was overly restrictive in counting PNG image chunks.
+  * CVE-2018-16336: remote denial of service (heap-based buffer over-read) via
+    a crafted image file.
+
+ -- Roberto C. Sanchez <roberto at debian.org>  Sat, 20 Oct 2018 22:43:10 -0400
+
 exiv2 (0.25-3.1+deb9u1) stretch-security; urgency=high
 
   * Non-maintainer upload by the Security Team.
diff --git a/debian/patches/CVE-2018-10958_10999_1_of_2.patch b/debian/patches/CVE-2018-10958_10999_1_of_2.patch
index 75f2ff2..4f227e6 100644
--- a/debian/patches/CVE-2018-10958_10999_1_of_2.patch
+++ b/debian/patches/CVE-2018-10958_10999_1_of_2.patch
@@ -32,7 +32,7 @@ There must be 2 null separators when we start to analyze the language tag.
          }
          else if(type == iTXt_Chunk)
          {
-+            const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_-1], '\0');
++            const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_], '\0');
 +            if (nullSeparators < 2) throw Error(58);
 +
              // Extract a deflate compressed or uncompressed UTF-8 text chunk
diff --git a/debian/patches/CVE-2018-10958_10999_2_of_2.patch b/debian/patches/CVE-2018-10958_10999_2_of_2.patch
index 75a2027..06e5ecd 100644
--- a/debian/patches/CVE-2018-10958_10999_2_of_2.patch
+++ b/debian/patches/CVE-2018-10958_10999_2_of_2.patch
@@ -14,7 +14,7 @@ Subject: [PATCH] Check validity of compressionFlag & compressionMethod
 @@ -159,14 +159,24 @@
          else if(type == iTXt_Chunk)
          {
-             const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_-1], '\0');
+             const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_], '\0');
 -            if (nullSeparators < 2) throw Error(58);
 +            if (nullSeparators < 2) throw Error(58, "iTXt chunk: not enough null separators");
  
diff --git a/debian/patches/CVE-2018-16336.patch b/debian/patches/CVE-2018-16336.patch
new file mode 100644
index 0000000..cfd63c5
--- /dev/null
+++ b/debian/patches/CVE-2018-16336.patch
@@ -0,0 +1,130 @@
+From 35b3e596edacd2437c2c5d3dd2b5c9502626163d Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Dan=20=C4=8Cerm=C3=A1k?= <dan.cermak at cgc-instruments.com>
+Date: Fri, 17 Aug 2018 16:41:05 +0200
+Subject: [PATCH] Add overflow & overread checks to PngChunk::parseTXTChunk()
+
+This function was creating a lot of new pointers and strings without
+properly checking the array bounds. This commit adds several calls
+to enforce(), making sure that the pointers stay within bounds.
+Strings are now created using the helper function
+string_from_unterminated() to prevent overreads in the constructor of
+std::string.
+
+This fixes #400
+---
+ src/pngchunk_int.cpp | 63 ++++++++++++++++++++++++++++++----------------------
+ 1 file changed, 37 insertions(+), 26 deletions(-)
+
+--- exiv2-stretch.git.orig/src/pngchunk.cpp
++++ exiv2-stretch.git/src/pngchunk.cpp
+@@ -40,6 +40,8 @@
+ #include "iptc.hpp"
+ #include "image.hpp"
+ #include "error.hpp"
++#include "helper_functions.hpp"
++#include "safe_op.hpp"
+ 
+ // + standard includes
+ #include <sstream>
+@@ -127,6 +129,8 @@
+ 
+         if(type == zTXt_Chunk)
+         {
++            if (data.size_ < Safe::add(keysize, 2)) throw Error(58);
++
+             // Extract a deflate compressed Latin-1 text chunk
+ 
+             // we get the compression method after the key
+@@ -143,11 +147,13 @@
+             // compressed string after the compression technique spec
+             const byte* compressedText      = data.pData_ + keysize + 2;
+             unsigned int compressedTextSize = data.size_  - keysize - 2;
++            if (compressedTextSize >= data.size_) throw Error(58);
+ 
+             zlibUncompress(compressedText, compressedTextSize, arr);
+         }
+         else if(type == tEXt_Chunk)
+         {
++            if (data.size_ < Safe::add(keysize, 1)) throw Error(58);
+             // Extract a non-compressed Latin-1 text chunk
+ 
+             // the text comes after the key, but isn't null terminated
+@@ -158,6 +164,7 @@
+         }
+         else if(type == iTXt_Chunk)
+         {
++            if (data.size_ < Safe::add(keysize, 3)) throw Error(58);
+             const int nullSeparators = std::count(&data.pData_[keysize+3], &data.pData_[data.size_], '\0');
+             if (nullSeparators < 2) throw Error(58, "iTXt chunk: not enough null separators");
+ 
+@@ -178,41 +185,43 @@
+             }
+ 
+             // language description string after the compression technique spec
+-            std::string languageText((const char*)(data.pData_ + keysize + 3));
+-            unsigned int languageTextSize = static_cast<unsigned int>(languageText.size());
+-            // translated keyword string after the language description
+-            std::string translatedKeyText((const char*)(data.pData_ + keysize + 3 + languageTextSize +1));
+-            unsigned int translatedKeyTextSize = static_cast<unsigned int>(translatedKeyText.size());
++            const size_t languageTextMaxSize = data.size_ - keysize - 3;
++            std::string languageText =
++                string_from_unterminated((const char*)(data.pData_ + Safe::add(keysize, 3)), languageTextMaxSize);
++            const unsigned int languageTextSize = static_cast<unsigned int>(languageText.size());
+ 
+-            if ( compressionFlag == 0x00 )
+-            {
+-                // then it's an uncompressed iTXt chunk
+-#ifdef DEBUG
+-                std::cout << "Exiv2::PngChunk::parseTXTChunk: We found an uncompressed iTXt field\n";
+-#endif
++            if (data.size_ < Safe::add(static_cast<unsigned int>(Safe::add(keysize, 4)), languageTextSize)) throw Error(58);
++            // translated keyword string after the language description
++            std::string translatedKeyText =
++                string_from_unterminated((const char*)(data.pData_ + keysize + 3 + languageTextSize + 1),
++                                         data.size_ - (keysize + 3 + languageTextSize + 1));
++            const unsigned int translatedKeyTextSize = static_cast<unsigned int>(translatedKeyText.size());
++
++            if ((compressionFlag == 0x00) || (compressionFlag == 0x01 && compressionMethod == 0x00)) {
++                if (Safe::add(static_cast<unsigned int>(keysize + 3 + languageTextSize + 1),
++                                  Safe::add(translatedKeyTextSize, 1u)) > data.size_) throw Error(58);
+ 
+-                // the text comes after the translated keyword, but isn't null terminated
+                 const byte* text = data.pData_ + keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1;
+-                long textsize    = data.size_ - (keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1);
++                const long textsize = data.size_ - (keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1);
+ 
+-                arr.alloc(textsize);
+-                arr = DataBuf(text, textsize);
+-            }
+-            else if ( compressionFlag == 0x01 && compressionMethod == 0x00 )
+-            {
+-                // then it's a zlib compressed iTXt chunk
++                if (compressionFlag == 0x00) {
++                    // then it's an uncompressed iTXt chunk
+ #ifdef DEBUG
+-                std::cout << "Exiv2::PngChunk::parseTXTChunk: We found a zlib compressed iTXt field\n";
++                    std::cout << "Exiv2::PngChunk::parseTXTChunk: We found an uncompressed iTXt field\n";
+ #endif
+ 
+-                // the compressed text comes after the translated keyword, but isn't null terminated
+-                const byte* compressedText = data.pData_ + keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1;
+-                long compressedTextSize    = data.size_ - (keysize + 3 + languageTextSize + 1 + translatedKeyTextSize + 1);
++                    arr.alloc(textsize);
++                    arr = DataBuf(text, textsize);
++                } else if (compressionFlag == 0x01 && compressionMethod == 0x00) {
++                    // then it's a zlib compressed iTXt chunk
++#ifdef DEBUG
++                    std::cout << "Exiv2::PngChunk::parseTXTChunk: We found a zlib compressed iTXt field\n";
++#endif
+ 
+-                zlibUncompress(compressedText, compressedTextSize, arr);
+-            }
+-            else
+-            {
++                    // the compressed text comes after the translated keyword, but isn't null terminated
++                    zlibUncompress(text, textsize, arr);
++                }
++            } else {
+                 // then it isn't zlib compressed and we are sunk
+ #ifdef DEBUG
+                 std::cerr << "Exiv2::PngChunk::parseTXTChunk: Non-standard iTXt compression method.\n";
diff --git a/debian/patches/CVE-2018-16336_prereq.patch b/debian/patches/CVE-2018-16336_prereq.patch
new file mode 100644
index 0000000..4a5db64
--- /dev/null
+++ b/debian/patches/CVE-2018-16336_prereq.patch
@@ -0,0 +1,105 @@
+--- /dev/null
++++ exiv2-stretch.git/src/helper_functions.cpp
+@@ -0,0 +1,38 @@
++// ********************************************************* -*- C++ -*-
++/*
++ * Copyright (C) 2004-2018 Exiv2 authors
++ * This program is part of the Exiv2 distribution.
++ *
++ * This program is free software; you can redistribute it and/or
++ * modify it under the terms of the GNU General Public License
++ * as published by the Free Software Foundation; either version 2
++ * of the License, or (at your option) any later version.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program; if not, write to the Free Software
++ * Foundation, Inc., 51 Franklin Street, 5th Floor, Boston, MA 02110-1301 USA.
++ */
++/*!
++  @file    helper_functions.cpp
++  @brief   A collection of helper functions
++  @author  Dan ?ermák (D4N)
++           <a href="mailto:dan.cermak at cgc-instruments.com">dan.cermak at cgc-instruments.com</a>
++  @date    25-May-18, D4N: created
++ */
++
++#include "helper_functions.hpp"
++
++#include <string.h>
++
++
++std::string string_from_unterminated(const char* data, size_t data_length)
++{
++    const size_t StringLength = strnlen(data, data_length);
++
++    return std::string(data, StringLength);
++}
+--- /dev/null
++++ exiv2-stretch.git/src/helper_functions.hpp
+@@ -0,0 +1,49 @@
++// ********************************************************* -*- C++ -*-
++/*
++ * Copyright (C) 2004-2018 Exiv2 authors
++ * This program is part of the Exiv2 distribution.
++ *
++ * This program is free software; you can redistribute it and/or
++ * modify it under the terms of the GNU General Public License
++ * as published by the Free Software Foundation; either version 2
++ * of the License, or (at your option) any later version.
++ *
++ * This program is distributed in the hope that it will be useful,
++ * but WITHOUT ANY WARRANTY; without even the implied warranty of
++ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
++ * GNU General Public License for more details.
++ *
++ * You should have received a copy of the GNU General Public License
++ * along with this program; if not, write to the Free Software
++ * Foundation, Inc., 51 Franklin Street, 5th Floor, Boston, MA 02110-1301 USA.
++ */
++/*!
++  @file    helper_functions.hpp
++  @brief   A collection of helper functions
++  @author  Dan ?ermák (D4N)
++           <a href="mailto:dan.cermak at cgc-instruments.com">dan.cermak at cgc-instruments.com</a>
++  @date    25-May-18, D4N: created
++ */
++#ifndef HELPER_FUNCTIONS_HPP
++#define HELPER_FUNCTIONS_HPP
++
++#include <string>
++
++/*!
++  @brief Convert a (potentially not null terminated) array into a
++  std::string.
++
++  Convert a C style string that may or may not be null terminated safely
++  into a std::string. The string's termination is either set at the first \0
++  or after data_length characters.
++
++  @param[in] data  A c-string from which the std::string shall be
++      constructed. Does not need to be null terminated.
++  @param[in] data_length  An upper bound for the string length (must be at most
++      the allocated length of `buffer`). If no null terminator is found in data,
++      then the resulting std::string will be null terminated at `data_length`.
++
++ */
++std::string string_from_unterminated(const char* data, size_t data_length);
++
++#endif  // HELPER_FUNCTIONS_HPP
+--- exiv2-stretch.git.orig/src/Makefile
++++ exiv2-stretch.git/src/Makefile
+@@ -120,7 +120,8 @@
+ 	 value.cpp             \
+ 	 version.cpp           \
+ 	 xmp.cpp               \
+-	 xmpsidecar.cpp
++	 xmpsidecar.cpp        \
++	 helper_functions.cpp
+ ifdef ENABLE_VIDEO
+ CCSRC += asfvideo.cpp          \
+ 	 matroskavideo.cpp     \
diff --git a/debian/patches/series b/debian/patches/series
index 613c9e6..8f36266 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -10,3 +10,5 @@ CVE-2018-11531_3_of_3.patch
 CVE-2018-12265_prereq.patch
 CVE-2018-12265.patch
 CVE-2018-12264.patch
+CVE-2018-16336_prereq.patch
+CVE-2018-16336.patch


More information about the pkg-kde-extras mailing list