[0.26] try backporting fixes for CVE-2018-{12264,12265}

src/exiv2.cpp

@@ -150,30 +150,36 @@ int main(int argc, char* const argv[])
         return 0;
     }
 
-    // Create the required action class
-    Action::TaskFactory& taskFactory = Action::TaskFactory::instance();
-    Action::Task::AutoPtr task
-        = taskFactory.create(Action::TaskType(params.action_));
-    assert(task.get());
-
-    // Process all files
     int rc = 0;
-    int n = 1;
-    int s = static_cast<int>(params.files_.size());
-    int w = s > 9 ? s > 99 ? 3 : 2 : 1;
-    for (Params::Files::const_iterator i = params.files_.begin();
-         i != params.files_.end(); ++i) {
-        if (params.verbose_) {
-            std::cout << _("File") << " " << std::setw(w) << std::right << n++ << "/" << s << ": "
-                      << *i << std::endl;
+
+    try {
+        // Create the required action class
+        Action::TaskFactory& taskFactory = Action::TaskFactory::instance();
+        Action::Task::AutoPtr task = taskFactory.create(Action::TaskType(params.action_));
+        assert(task.get());
+
+        // Process all files
+        int n = 1;
+        int s = static_cast<int>(params.files_.size());
+        int w = s > 9 ? s > 99 ? 3 : 2 : 1;
+        for (Params::Files::const_iterator i = params.files_.begin(); i != params.files_.end(); ++i) {
+            if (params.verbose_) {
+                std::cout << _("File") << " " << std::setw(w) << std::right << n++ << "/" << s << ": " << *i
+                          << std::endl;
+            }
+            int ret = task->run(*i);
+            if (rc == 0)
+                rc = ret;
         }
-        int ret = task->run(*i);
-        if (rc == 0) rc = ret;
-    }
 
-    taskFactory.cleanup();
-    params.cleanup();
-    Exiv2::XmpParser::terminate();
+        taskFactory.cleanup();
+        params.cleanup();
+        Exiv2::XmpParser::terminate();
+
+    } catch (const std::exception& exc) {
+        std::cerr << "Uncaught exception: " << exc.what() << std::endl;
+        rc = 1;
+    }
 
     // Return a positive one byte code for better consistency across platforms
     return static_cast<unsigned int>(rc) % 256;

src/preview.cpp

@@ -36,6 +36,7 @@ EXIV2_RCSID("@(#) $Id$")
 
 #include "preview.hpp"
 #include "futils.hpp"
+#include "safe_op.hpp"
 
 #include "image.hpp"
 #include "cr2image.hpp"
@@ -547,7 +548,8 @@ namespace {
             }
         }
 
-        if (offset_ + size_ > static_cast<uint32_t>(image_.io().size())) return;
+        if (Safe::add(offset_, size_) > static_cast<uint32_t>(image_.io().size()))
+            return;
 
         valid_ = true;
     }
@@ -801,7 +803,7 @@ namespace {
                     // this saves one copying of the buffer
                     uint32_t offset = dataValue.toLong(0);
                     uint32_t size = sizes.toLong(0);
-                    if (offset + size <= static_cast<uint32_t>(io.size()))
+                    if (Safe::add(offset, size) <= static_cast<uint32_t>(io.size()))
                         dataValue.setDataArea(base + offset, size);
                 }
                 else {
@@ -811,7 +813,7 @@ namespace {
                     for (int i = 0; i < sizes.count(); i++) {
                         uint32_t offset = dataValue.toLong(i);
                         uint32_t size = sizes.toLong(i);
-                        if (offset + size <= static_cast<uint32_t>(io.size()))
+                        if (Safe::add(offset, size) <= static_cast<uint32_t>(io.size()))
                             memcpy(pos, base + offset, size);
                         pos += size;
                     }

tests/bugfixes/github/test_CVE_2018_12264.py

@@ -0,0 +1,21 @@
+# -*- coding: utf-8 -*-
+
+import system_tests
+
+
+class AdditionOverflowInLoaderTiffGetData(metaclass=system_tests.CaseMeta):
+    """
+    Regression test for bug #366:
+    https://github.com/Exiv2/exiv2/issues/366
+    aka CVE-2018-12264
+    https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-12264
+    """
+    filename = "$data_path/2-out-of-read-Poc"
+    commands = ["$exiv2 -ep $filename"]
+    stdout = [""]
+    stderr = [
+        """Warning: Directory Image, entry 0x0111: Strip 0 is outside of the data area; ignored.
+$uncaught_exception Overflow in addition
+"""
+    ]
+    retval = [1]

tests/bugfixes/github/test_CVE_2018_12265.py

@@ -0,0 +1,24 @@
+# -*- coding: utf-8 -*-
+
+import system_tests
+
+
+class AdditionOverflowInLoaderExifJpeg(metaclass=system_tests.CaseMeta):
+    """
+    Regression test for bug #365:
+    https://github.com/Exiv2/exiv2/issues/365
+    aka CVE 2018-12265:
+    https://cve.mitre.org/cgi-bin/cvename.cgi?name=2018-12265
+    """
+    filename = "$data_path/1-out-of-read-Poc"
+    commands = ["$exiv2 -ep $filename"]
+    stdout = [""]
+    stderr = [
+        """Error: Upper boundary of data for directory Image, entry 0x00fe is out of bounds: Offset = 0x0000002a, size = 64, exceeds buffer size by 22 Bytes; truncating the entry
+Warning: Directory Image, entry 0x0201: Strip 0 is outside of the data area; ignored.
+Warning: Directory Image, entry 0x0201: Strip 7 is outside of the data area; ignored.
+Error: Offset of directory Thumbnail, entry 0x0201 is out of bounds: Offset = 0x00000000; truncating the entry
+$uncaught_exception Overflow in addition
+"""
+    ]
+    retval = [1]

tests/suite.conf

@@ -19,3 +19,4 @@ error_14_message: Failed to read image data
 error_58_message: corrupted image metadata
 error_57_message: invalid memory allocation request
 exiv2_exception_msg: Exiv2 exception in print action for file
+uncaught_exception: Uncaught exception:

tests/system_tests.py

@@ -8,6 +8,7 @@
 import shlex
 import sys
 import shutil
+import string
 import unittest
 
 
@@ -50,6 +51,7 @@ def _disjoint_dict_merge(d1, d2):
     return res
 
 
+#: global parameters extracted from the test suite's configuration file
 _parameters = {}
 
 
@@ -210,42 +212,75 @@ def wrapper(cls):
     return wrapper
 
 
+def test_run(self):
+    """
+    This function reads in the members commands, retval, stdout, stderr and runs
+    the `expand_variables` function on each. The resulting commands are then run
+    using the subprocess module and compared against the expected values that
+    were provided in the static members via `compare_stdout` and
+    `compare_stderr`. Furthermore a threading.Timer is used to abort the
+    execution if a configured timeout is reached.
+
+    It is automatically added as a member function to each system test by the
+    CaseMeta metaclass. This ensures that it is run by each system test
+    **after** setUp() and setUpClass() were run.
+    """
+    for i, command, retval, stdout, stderr in zip(range(len(self.commands)),
+                                                  self.commands,
+                                                  self.retval,
+                                                  self.stdout,
+                                                  self.stderr):
+        command, retval, stdout, stderr = map(
+            self.expand_variables, [command, retval, stdout, stderr]
+        )
+        retval = int(retval)
+        timeout = {"flag": False}
+
+        proc = subprocess.Popen(
+            _cmd_splitter(command),
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            cwd=self.work_dir
+        )
+
+        def timeout_reached(timeout):
+            timeout["flag"] = True
+            proc.kill()
+
+        t = threading.Timer(
+            _parameters["timeout"], timeout_reached, args=[timeout]
+        )
+        t.start()
+        got_stdout, got_stderr = proc.communicate()
+        t.cancel()
+
+        processed_stdout = _process_output_post(got_stdout.decode('utf-8'))
+        processed_stderr = _process_output_post(got_stderr.decode('utf-8'))
+
+        self.assertFalse(timeout["flag"] and "Timeout reached")
+        self.compare_stdout(i, command, processed_stdout, stdout)
+        self.compare_stderr(i, command, processed_stderr, stderr)
+        self.assertEqual(retval, proc.returncode)
+
+
 class Case(unittest.TestCase):
     """
     System test case base class, provides the functionality to interpret static
-    class members as system tests and runs them.
-
-    This class reads in the members commands, retval, stdout, stderr and runs
-    the format function on each, where format is called with the kwargs being a
-    merged dictionary of all variables that were extracted from the suite's
-    configuration file and all static members of the current class.
-
-    The resulting commands are then run using the subprocess module and compared
-    against the expected values that were provided in the static
-    members. Furthermore a threading.Timer is used to abort the execution if a
-    configured timeout is reached.
-
-    The class itself must be inherited from, otherwise it is not useful at all,
-    as it does not provide any static members that could be used to run system
-    tests. However, a class that inherits from this class needn't provide any
-    member functions at all, the inherited test_run() function performs all
-    required functionality in child classes.
+    class members as system tests.
+
+    The class itself only provides utility functions and system tests need not
+    inherit from it, as it is automatically added via the CaseMeta metaclass.
     """
 
-    """ maxDiff set so that arbitrarily large diffs will be shown """
+    #: maxDiff set so that arbitrarily large diffs will be shown
     maxDiff = None
 
     @classmethod
     def setUpClass(cls):
         """
-        This function adds the variables variable_dict & work_dir to the class.
-
-        work_dir - set to the file where the current class is defined
-        variable_dict - a merged dictionary of all static members of the current
-                        class and all variables extracted from the suite's
-                        configuration file
+        This function adds the variable work_dir to the class, which is the path
+        to the directory where the python source file is located.
         """
-        cls.variable_dict = _disjoint_dict_merge(cls.__dict__, _parameters)
         cls.work_dir = os.path.dirname(inspect.getfile(cls))
 
     def compare_stdout(self, i, command, got_stdout, expected_stdout):
@@ -267,65 +302,79 @@ def compare_stdout(self, i, command, got_stdout, expected_stdout):
         self.assertMultiLineEqual(expected_stdout, got_stdout)
 
     def compare_stderr(self, i, command, got_stderr, expected_stderr):
-        """
-        Same as compare_stdout only for standard-error.
-        """
+        """ Same as compare_stdout only for standard-error. """
         self.assertMultiLineEqual(expected_stderr, got_stderr)
 
-    def expand_variables(self, string):
+    def expand_variables(self, unexpanded_string):
         """
-        Expands all variables in curly braces in the given string using the
-        dictionary variable_dict.
+        Expands all variables of the form ``$var`` in the given string using the
+        dictionary `variable_dict`.
 
-        The expansion itself is performed by the builtin string method format().
-        A KeyError indicates that the supplied string contains a variable
-        in curly braces that is missing from self.variable_dict
+        The expansion itself is performed by the string's template module using
+        via `safe_substitute`.
         """
-        return str(string).format(**self.variable_dict)
+        return string.Template(str(unexpanded_string))\
+            .safe_substitute(**self.variable_dict)
 
-    def test_run(self):
-        """
-        Actual system test function which runs the provided commands,
-        pre-processes all variables and post processes the output before passing
-        it on to compare_stderr() & compare_stdout().
-        """
 
-        for i, command, retval, stdout, stderr in zip(range(len(self.commands)),
-                                                      self.commands,
-                                                      self.retval,
-                                                      self.stdout,
-                                                      self.stderr):
-            command, retval, stdout, stderr = map(
-                self.expand_variables, [command, retval, stdout, stderr]
-            )
-            retval = int(retval)
-            timeout = {"flag": False}
-
-            proc = subprocess.Popen(
-                _cmd_splitter(command),
-                stdout=subprocess.PIPE,
-                stderr=subprocess.PIPE,
-                cwd=self.work_dir
-            )
+class CaseMeta(type):
+    """ System tests generation metaclass.
 
-            def timeout_reached(timeout):
-                timeout["flag"] = True
-                proc.kill()
+    This metaclass is performs the following tasks:
 
-            t = threading.Timer(
-                _parameters["timeout"], timeout_reached, args=[timeout]
-            )
-            t.start()
-            got_stdout, got_stderr = proc.communicate()
-            t.cancel()
-
-            self.assertFalse(timeout["flag"] and "Timeout reached")
-            self.compare_stdout(
-                i, command,
-                _process_output_post(got_stdout.decode('utf-8')), stdout
-            )
-            self.compare_stderr(
-                i, command,
-                _process_output_post(got_stderr.decode('utf-8')), stderr
-            )
-            self.assertEqual(retval, proc.returncode)
+    1. Add the `test_run` function as a member of the test class
+    2. Add the `Case` class as the parent class
+    3. Expand all variables already defined in the class, so that any additional
+       code does not have to perform this task
+
+    Using a metaclass instead of inheriting from case has the advantage, that we
+    can expand all variables in the strings before any test case or even the
+    class constructor is run! Thus users will immediately see the expanded
+    result. Also adding the `test_run` function as a direct member and not via
+    inheritance enforces that it is being run **after** the test cases setUp &
+    setUpClass (which oddly enough seems not to be the case in the unittest
+    module where test functions of the parent class run before setUpClass of the
+    child class).
+    """
+
+    def __new__(mcs, clsname, bases, dct):
+
+        changed = False
+
+        # expand all non-private variables by brute force
+        # => try all expanding all elements defined in the current class until
+        # there is no change in them any more
+        keys = [key for key in list(dct.keys()) if not key.startswith('_')]
+        while not changed:
+            for key in keys:
+
+                old_value = dct[key]
+
+                # only try expanding strings and lists
+                if isinstance(old_value, str):
+                    new_value = string.Template(old_value).safe_substitute(
+                        **_disjoint_dict_merge(dct, _parameters)
+                    )
+                elif isinstance(old_value, list):
+                    # do not try to expand anything but strings in the list
+                    new_value = [
+                        string.Template(elem).safe_substitute(
+                            **_disjoint_dict_merge(dct, _parameters)
+                        )
+                        if isinstance(elem, str) else elem
+                        for elem in old_value
+                    ]
+                else:
+                    continue
+
+                if old_value != new_value:
+                    changed = True
+                    dct[key] = new_value
+
+        dct['variable_dict'] = _disjoint_dict_merge(dct, _parameters)
+        dct['test_run'] = test_run
+
+        if Case not in bases:
+            bases += (Case,)
+
+        return super(CaseMeta, mcs).__new__(mcs, clsname, bases, dct)