[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Debian-med-packaging] Wheezy update of dcmtk?



On 12/19/2016 03:58 PM, Bálint Réczey wrote:
> Hi,
> 
> 2016-12-19 9:10 GMT+01:00 Sébastien Jodogne <s.jodogne@gmail.com>:
>> Dear all,
>>
>>> On Sun, Dec 18, 2016 at 10:47:05PM +0100, Markus Koschany wrote:
>>>> Hello dear maintainer(s),
>>>>
>>>> the Debian LTS team would like to fix the security issues which are
>>>> currently open in the Wheezy version of dcmtk:
>>>> https://security-tracker.debian.org/tracker/CVE-2015-8979
>>>>
>>>> Would you like to take care of this yourself?
>>>
>>> I personally feel not capable to do so and Mathieu left the team - so I
>>> would be astonished (but definitely happy!) if he would step in for this
>>> task.  If you do not receive a positive response from Gert I doubt that
>>> anybody else from the team would take over.
>>
>>
>> I personally consider this issue as severe, as any DCMTK 3.6.0-based DICOM
>> SCP (server) is affected (including the well-known Horos/OsiriX viewer).
>>
>> Orthanc was also affected by this problem. Orthanc 1.2.0 was released last
>> week in order to fix this vulnerability in its static builds (notably for
>> Windows and OS X). The patch we applied can be found at the following
>> location:
>> https://bitbucket.org/sjodogne/orthanc/src/eb363ec95d863989abf5a59174ff3164c2831f2e/Resources/Patches/dcmtk-3.6.0-dulparse-vulnerability.patch?at=default&fileviewer=file-view-default
>>
>> As this patch is very simple (six lines of code), it should be easy to
>> backport it to the DCMTK Debian package.
>>
>> Unfortunately, I do not know how to fix such issues in Wheezy, and I am
>> currently under heavy pressure wrt. the Orthanc upstream project... maybe
>> someone could do this backporting job?
> 
> I'll do it in a few hours.
> I have also claimed the package in dla-needed.txt.

Thank you for the additional info and the potential patch.

I have prepared the update for Wheezy based on the upstream patch
instead to diverge less from upstream in case we have to patch the code
further. The error reporting is also more verbose and accurate.

Please see the diff to previous version attached.

Changes:
 dcmtk (3.6.0-12+deb7u1) wheezy-security; urgency=medium
 .
   * LTS Team upload.
   * Fix remote stack buffer overflow (CVE-2015-8979) (Closes: #848830)
   * Enable tests for the fix

I plan uploading the package today around 22:00 UTC.

The binary packages for amd64 are also available for testing here:

 deb https://people.debian.org/~rbalint/ppa/wheezy-lts UNRELEASED/

Cheers,
Balint

diff -Nru dcmtk-3.6.0/debian/changelog dcmtk-3.6.0/debian/changelog
--- dcmtk-3.6.0/debian/changelog	2012-12-20 13:22:26.000000000 +0100
+++ dcmtk-3.6.0/debian/changelog	2016-12-20 03:23:36.000000000 +0100
@@ -1,3 +1,11 @@
+dcmtk (3.6.0-12+deb7u1) wheezy-security; urgency=medium
+
+  * LTS Team upload.
+  * Fix remote stack buffer overflow (CVE-2015-8979) (Closes: #848830)
+  * Enable tests for the fix
+
+ -- Balint Reczey <balint@balintreczey.hu>  Mon, 19 Dec 2016 20:41:08 +0100
+
 dcmtk (3.6.0-12) unstable; urgency=low
 
   [ Andrey Rahmatullin ]
diff -Nru dcmtk-3.6.0/debian/patches/0001-Fixed-possible-underflows-and-overflows.patch dcmtk-3.6.0/debian/patches/0001-Fixed-possible-underflows-and-overflows.patch
--- dcmtk-3.6.0/debian/patches/0001-Fixed-possible-underflows-and-overflows.patch	1970-01-01 01:00:00.000000000 +0100
+++ dcmtk-3.6.0/debian/patches/0001-Fixed-possible-underflows-and-overflows.patch	2016-12-20 16:47:41.000000000 +0100
@@ -0,0 +1,977 @@
+From 5475a01c74bdf6258eccd4238e5df42eaca8ba58 Mon Sep 17 00:00:00 2001
+From: Michael Onken <onken@open-connections.de>
+Date: Mon, 14 Dec 2015 21:50:43 +0100
+Subject: [PATCH] Fixed possible underflows and overflows.
+
+At several places in the code a wrong length of ACSE data structures received
+over the network can cause overflows or underflows when processing those
+data structures. Related checks have been added at various places in order
+to prevent such (possible) attacks.
+
+Thanks to Kevin Basista for the report.
+
+Conflicts:
+	dcmnet/libsrc/dulparse.cc
+	ofstd/tests/tests.cc
+	ofstd/tests/tofstd.cc
+
+Cutting safeAdd() to not pull in many new files by Balint Reczey.
+---
+ dcmnet/libsrc/dulparse.cc           |  93 +++++--
+ ofstd/include/dcmtk/ofstd/ofstd.h   |  27 +-
+ ofstd/include/dcmtk/ofstd/oftest.h  | 519 ++++++++++++++++++++++++++++++++++++
+ ofstd/tests/Makefile.in             |  15 +-
+ ofstd/tests/taddsub.cc              |  47 ++++
+ ofstd/tests/tests-new-framework.exp |   1 +
+ ofstd/tests/tests.cc                |  28 ++
+ 7 files changed, 705 insertions(+), 25 deletions(-)
+ create mode 100644 ofstd/include/dcmtk/ofstd/oftest.h
+ create mode 100644 ofstd/tests/taddsub.cc
+ create mode 100644 ofstd/tests/tests-new-framework.exp
+ create mode 100644 ofstd/tests/tests.cc
+
+diff --git a/dcmnet/libsrc/dulparse.cc b/dcmnet/libsrc/dulparse.cc
+index ee6a7bd..1a9f4f4 100644
+--- a/dcmnet/libsrc/dulparse.cc
++++ b/dcmnet/libsrc/dulparse.cc
+@@ -1,6 +1,6 @@
+ /*
+  *
+- *  Copyright (C) 1994-2010, OFFIS e.V.
++ *  Copyright (C) 1994-2015, OFFIS e.V.
+  *  All rights reserved.  See COPYRIGHT file for details.
+  *
+  *  This software and supporting documentation were partly developed by
+@@ -72,6 +72,7 @@
+ #include "dcmtk/ofstd/ofstdinc.h"
+ 
+ #include "dcmtk/ofstd/ofstream.h"
++#include "dcmtk/ofstd/ofstd.h"  // for OFStandard::safeSubtract() and safeAdd()
+ #include "dcmtk/dcmnet/dicom.h"
+ #include "dcmtk/dcmnet/cond.h"
+ #include "dcmtk/dcmnet/lst.h"
+@@ -112,6 +113,9 @@ static OFCondition
+ makeLengthError(const char *pdu, unsigned long bufSize, unsigned long minSize = 0,
+         unsigned long length = 0);
+ 
++static OFCondition
++makeUnderflowError(const char *pdu, unsigned long minuend, unsigned long subtrahend);
++
+ /* parseAssociate
+ **
+ ** Purpose:
+@@ -122,7 +126,7 @@ makeLengthError(const char *pdu, unsigned long bufSize, unsigned long minSize =
+ **      buf             Buffer holding the PDU in the stream format
+ **      pduLength       Length of the buffer
+ **      assoc           The Associate PDU to be extracted
+-**                      (returend to the caller)
++**                      (returned to the caller)
+ **
+ ** Return Values:
+ **
+@@ -240,6 +244,8 @@ parseAssociate(unsigned char *buf, unsigned long pduLength,
+             DCMNET_TRACE("Successfully parsed Presentation Context");
+             break;
+         case DUL_TYPEUSERINFO:
++            // parse user info, which can contain several sub-items like User
++            // Identity Negotiation or SOP Class Extended Negotiation
+             cond = parseUserInfo(&assoc->userInfo, buf, &itemLength, assoc->type, pduLength);
+             if (cond.bad())
+                 return cond;
+@@ -250,7 +256,8 @@ parseAssociate(unsigned char *buf, unsigned long pduLength,
+         default:
+             cond = parseDummy(buf, &itemLength, pduLength);
+             buf += itemLength;
+-            pduLength -= itemLength;
++            if (!OFStandard::safeSubtract(pduLength, itemLength, pduLength))
++              return makeUnderflowError("unknown item type", pduLength, itemLength);
+             break;
+         }
+     }
+@@ -410,7 +417,8 @@ parsePresentationContext(unsigned char type,
+             default:
+                 cond = parseDummy(buf, &length, presentationLength);
+                 buf += length;
+-                presentationLength -= length;
++                if (!OFStandard::safeSubtract(presentationLength, length, presentationLength))
++                  return makeUnderflowError("unknown presentation context type", presentationLength, length);
+                 break;
+             }
+         }
+@@ -445,7 +453,7 @@ parseUserInfo(DUL_USERINFO * userInfo,
+               unsigned char *buf,
+               unsigned long *itemLength,
+               unsigned char typeRQorAC,
+-              unsigned long availData)
++              unsigned long availData /* bytes left for in this PDU */)
+ {
+     unsigned short userLength;
+     unsigned long length;
+@@ -454,25 +462,34 @@ parseUserInfo(DUL_USERINFO * userInfo,
+     SOPClassExtendedNegotiationSubItem *extNeg = NULL;
+     UserIdentityNegotiationSubItem *usrIdent = NULL;
+ 
+-    // The minimum allowed size is 4 byte, else we read past the buffer end
++    // minimum allowed size is 4 byte (case where the length of the user data is 0),
++    // else we read past the buffer end
+     if (availData < 4)
+         return makeLengthError("user info", availData, 4);
+ 
++    // skip item type (50H) field
+     userInfo->type = *buf++;
++    // skip unused ("reserved") field
+     userInfo->rsv1 = *buf++;
++    // get and remeber announced length of user data
+     EXTRACT_SHORT_BIG(buf, userInfo->length);
++    // .. and skip over the two length field bytes
+     buf += 2;
+ 
++    // userLength contains announced length of full user item structure,
++    // will be used here to count down the available data later
+     userLength = userInfo->length;
++    // itemLength contains full length of the user item including the 4 bytes extra header (type, reserved + 2 for length)
+     *itemLength = userLength + 4;
+ 
+-    // Does this item claim to be larger than the available data?
+-    if (availData - 4 < userLength)
++    // does this item claim to be larger than the available data?
++    if (availData < *itemLength)
+         return makeLengthError("user info", availData, 0, userLength);
+ 
+     DCMNET_TRACE("Parsing user info field ("
+             << STD_NAMESPACE hex << STD_NAMESPACE setfill('0') << STD_NAMESPACE setw(2) << (unsigned int)userInfo->type
+             << STD_NAMESPACE dec << "), Length: " << (unsigned long)userInfo->length);
++    // parse through different types of user items as long as we have data
+     while (userLength > 0) {
+         DCMNET_TRACE("Parsing remaining " << (long)userLength << " bytes of User Information" << OFendl
+                 << "Next item type: "
+@@ -498,7 +515,8 @@ parseUserInfo(DUL_USERINFO * userInfo,
+         case DUL_TYPEASYNCOPERATIONS:
+             cond = parseDummy(buf, &length, userLength);
+             buf += length;
+-            userLength -= (unsigned short) length;
++            if (!OFStandard::safeSubtract(userLength, OFstatic_cast(short unsigned int, length), userLength))
++              return makeLengthError("asynchronous operation user item type", userLength, length);
+             break;
+         case DUL_TYPESCUSCPROLE:
+             role = (PRV_SCUSCPROLE*)malloc(sizeof(PRV_SCUSCPROLE));
+@@ -551,9 +569,14 @@ parseUserInfo(DUL_USERINFO * userInfo,
+           userLength -= (unsigned short) length;
+           break;
+         default:
+-            cond = parseDummy(buf, &length, userLength);
++            // we hit an unknown user item that is not defined in the standard
++            // or still unknown to DCMTK
++            cond = parseDummy(buf, &length /* returns bytes "handled" by parseDummy */, userLength /* data available in bytes for user item */);
++            // skip the bytes read
+             buf += length;
+-            userLength -= (unsigned short) length;
++            // subtract bytes of parsed data from available data bytes
++            if (!OFStandard::safeSubtract(userLength, OFstatic_cast(unsigned short, length), userLength))
++              return makeUnderflowError("unknown user item", userLength, length);
+             break;
+         }
+     }
+@@ -610,8 +633,9 @@ parseMaxPDU(DUL_MAXLENGTH * max, unsigned char *buf,
+ **      User Length
+ **
+ ** Parameter Dictionary:
+-**      buf             The buffer that is to be parsed
+-**      itemLength      Length of structure extracted.
++**      buf             The buffer that is to be parsed (input/output value)
++**      itemLength      Length of structure extracted (output value)
++*       availData       Number of bytes announced to be available for this sub item (input value)
+ **
+ ** Return Values:
+ **
+@@ -623,23 +647,26 @@ parseMaxPDU(DUL_MAXLENGTH * max, unsigned char *buf,
+ static OFCondition
+ parseDummy(unsigned char *buf, unsigned long *itemLength, unsigned long availData)
+ {
+-    unsigned short
+-        userLength;
+-
+     // Is there enough data for the length field?
+     if (availData < 4)
+         return makeLengthError("dummy item", availData, 4);
+ 
++    // Get announced length of this sub-item and skip over the bytes read
++    // 1 byte item-type (e.g. 58H for User Identity Negotiation), 1 byte reserved,
++    // and 2 bytes length field
++    unsigned short userLength;
+     buf++;
+     buf++;
+     EXTRACT_SHORT_BIG(buf, userLength);
+     buf += 2;
+ 
++    // Return full length (announced + 4 extra bytes)
++    *itemLength = userLength + 4;
++
+     // Is there less data than the length field claims there is?
+     if (availData - 4 < userLength)
+         return makeLengthError("dummy item", availData, 0, userLength);
+ 
+-    *itemLength = userLength + 4;
+     return EC_Normal;
+ }
+ 
+@@ -688,8 +715,11 @@ parseSCUSCPRole(PRV_SCUSCPROLE * role, unsigned char *buf,
+     if (role->length - 4 < UIDLength)
+         return makeLengthError("SCU-SCP role list UID", role->length, 0, UIDLength);
+ 
+-    (void) memcpy(role->SOPClassUID, buf, UIDLength);
+-    role->SOPClassUID[UIDLength] = '\0';
++    OFStandard::strlcpy(role->SOPClassUID, (char*)buf, sizeof(role->SOPClassUID));
++    if (UIDLength > sizeof(role->SOPClassUID))
++    {
++      DCMNET_WARN("Provided role SOP Class UID " << role->SOPClassUID << " is shorter than its announced length " << UIDLength << " (ignored)");
++    }
+     buf += UIDLength;
+     role->SCURole = *buf++;
+     role->SCPRole = *buf++;
+@@ -801,6 +831,31 @@ makeLengthError(const char *pdu, unsigned long bufSize, unsigned long minSize,
+     return ret;
+ }
+ 
++/* makeUnderflowError
++ *
++ * This function is used to generate the OFCondition code if an underflow
++ * computation has been detected.
++ *
++ * @param pdu The name of the field or PDU which caused the invalid computation
++ * @param minuend The field (probably length) subtracted from
++ * @param subtrahend The number subtracted from minuend
++ */
++static OFCondition
++makeUnderflowError(const char *pdu, unsigned long minuend,
++        unsigned long subtrahend)
++{
++  OFStringStream stream;
++  stream << "DUL Illegal " << pdu << ". Got " << minuend << " bytes of data and told to subtrat " << subtrahend << " bytes of data";
++  stream << "." << OFStringStream_ends;
++
++  OFCondition ret;
++  OFSTRINGSTREAM_GETSTR(stream, tmpString)
++  ret = makeDcmnetCondition(DULC_INCORRECTBUFFERLENGTH, OF_error, tmpString);
++  OFSTRINGSTREAM_FREESTR(tmpString)
++  return ret;
++}
++
++
+ /* trim_trailing_spaces
+ **
+ ** Purpose:
+diff --git a/ofstd/include/dcmtk/ofstd/ofstd.h b/ofstd/include/dcmtk/ofstd/ofstd.h
+index 1354b36..05cdce5 100644
+--- a/ofstd/include/dcmtk/ofstd/ofstd.h
++++ b/ofstd/include/dcmtk/ofstd/ofstd.h
+@@ -556,15 +556,38 @@ class OFStandard
+      */
+     static long getProcessID();
+ 
+-     /** check whether the addition of two 32-bit integers yields in an overflow
++    /** check whether the addition of two 32-bit integers yields in an overflow
+      *  @param summand1 first integer value to be added
+      *  @param summand2 second integer value to be added
+      *  @return OFTrue if an overflow occurred during the addition, OFFalse otherwise
+      */
+     static inline OFBool check32BitAddOverflow(const Uint32 summand1,
+                                                const Uint32 summand2)
++     {
++       return (0xffffffff - summand1 < summand2);
++     }
++
++    /** check whether subtraction is safe (i.e. no underflow occurs) and if so,
++     *  perform it (i.e. compute minuend-subtrahend=difference). Only works for
++     *  unsigned types.
++     *  @param minuend number to subtract from
++     *  @param subtrahend number to subtract from minuend
++     *  @param difference difference, if subraction is safe, otherwise the
++     *    parameter value is not touched by the function
++     *  @return OFTrue if subtraction is safe and could be performed, OFFalse
++     *   otherwise
++     */
++    template <typename T>
++    static OFBool __attribute__ ((visibility("default")))
++    safeSubtract(T minuend, T subtrahend, T& difference)
+     {
+-      return (0xffffffff - summand1 < summand2);
++      assert ((minuend >= 0) && (subtrahend >= 0)); // assert(!OFnumeric_limits<T>::is_signed);
++      if (minuend < subtrahend) {
++        return OFFalse;
++      } else {
++        difference = minuend - subtrahend;
++        return OFTrue;
++      }
+     }
+ 
+  private:
+diff --git a/ofstd/include/dcmtk/ofstd/oftest.h b/ofstd/include/dcmtk/ofstd/oftest.h
+new file mode 100644
+index 0000000..9a724e2
+--- /dev/null
++++ b/ofstd/include/dcmtk/ofstd/oftest.h
+@@ -0,0 +1,519 @@
++/*
++ *
++ *  Copyright (C) 2011-2012, OFFIS e.V.
++ *  All rights reserved.  See COPYRIGHT file for details.
++ *
++ *  This code is inspired by quicktest.
++ *    Copyright (C) 2005-2008
++ *    Tyler Streeter (http://www.tylerstreeter.net)
++ *      http://quicktest.sourceforge.net
++ *
++ *  This software and supporting documentation were developed by
++ *
++ *    OFFIS e.V.
++ *    R&D Division Health
++ *    Escherweg 2
++ *    D-26121 Oldenburg, Germany
++ *
++ *
++ *  Module:  ofstd
++ *
++ *  Author:  Uli Schlachter
++ *
++ *  Purpose: Provide a test framework for the toolkit
++ *
++ */
++
++
++#ifndef OFTEST_H
++#define OFTEST_H
++
++#include "dcmtk/config/osconfig.h"
++#include "dcmtk/ofstd/ofconapp.h"   /* for OFCommandLine */
++#include "dcmtk/ofstd/ofconsol.h"   /* for CERR */
++#include "dcmtk/ofstd/oflist.h"     /* for class OFList */
++#include "dcmtk/ofstd/ofstream.h"   /* for class OFOStringStream */
++#include "dcmtk/ofstd/ofstring.h"   /* for class OFString */
++#include "dcmtk/ofstd/oftypes.h"    /* for OFBool */
++
++#ifdef OFTEST_OFSTD_ONLY
++
++#define OFTEST_LOG_VERBOSE(msg) do { \
++    if (verbose_) \
++        COUT << msg << OFendl; \
++} while (0)
++
++#else
++
++#include "dcmtk/oflog/oflog.h"
++
++static OFLogger testLogger = OFLog::getLogger("dcmtk.test");
++#define OFTEST_LOG_VERBOSE(msg) OFLOG_INFO(testLogger, msg)
++
++#endif
++
++/** @file oftest.h
++ *  A simple framework for writing and running test cases.
++ */
++
++/** A single test case which can be run */
++class OFTestTest
++{
++public:
++    /// This is the type used for test results.
++    typedef OFList<OFString> TestResult;
++
++    /** Special flags that a test can have. The flags for a test are the result
++     *  of a bitwise or of these individual flags.
++     */
++    enum E_Flags {
++        EF_None = 0x0,
++        /// Slow test which should only be run in exhaustive mode.
++        EF_Slow = 0x1
++    };
++
++    /** Contructor
++     *  @param testName the name of this test case
++     */
++    OFTestTest(const OFString& testName, int flag)
++      : testName_(testName)
++      , results_()
++      , flags_(flag)
++    {
++    }
++
++    /// Destructor
++    virtual ~OFTestTest()
++    {
++    }
++
++    /// @return the flags of this test case
++    int flags() const { return flags_; }
++
++    /// @return the name of this test case
++    const OFString& getTestName() const { return testName_; }
++
++    /** Execute this test case.
++     *  @return Reference to list of errors.
++     */
++    const TestResult& runAndReturn()
++    {
++        results_.clear();
++        run();
++        return results_;
++    }
++
++    /** Execute this test case.
++     *  @param result the list of error messages generated by this test.
++     *  @see #OFCHECK(condition)
++     */
++    virtual void run() = 0;
++
++    /** Add a new failure to the result set.
++     *  @param result list of test failures
++     *  @param file filename for this failure
++     *  @param line line number for this failure
++     *  @param message error description.
++     */
++    void recordFailure(const OFString& file, unsigned long int line, const OFString& message)
++    {
++        OFOStringStream oss;
++        oss << "FAILED test '" << testName_ << "' at " << file << ":" << line
++            << ": " << message << OFStringStream_ends;
++        OFSTRINGSTREAM_GETOFSTRING(oss, str)
++        results_.push_back(str);
++    }
++
++private:
++    /// The unique name of this test.
++    OFString testName_;
++
++    /// The test results, empty for success.
++    TestResult results_;
++
++    /// Flags that this test has.
++    const int flags_;
++};
++
++/** The test manager singleton manages the list of available test cases
++ *  and executes them.
++ */
++class OFTestManager
++{
++public:
++    /// @return the only instance of the test manager
++    static OFTestManager& instance()
++    {
++        static OFTestManager manager;
++        return manager;
++    }
++
++    /// @return the currently running test case
++    OFTestTest& currentTest()
++    {
++        if (!curTest_)
++            abort();
++        return *curTest_;
++    }
++
++    /** Register a test with this test manager.
++     *  @param test the test to register
++     */
++    void addTest(OFTestTest* test)
++    {
++        tests_.push_back(test);
++    }
++
++    /** Run a list of test cases. The results will be printed on the console.
++     *  @param tests tests to execute
++     */
++    int runTests(const OFList<OFTestTest*>& tests, const char *module)
++    {
++        unsigned int numFailed = 0;
++        OFListConstIterator(OFTestTest*) it;
++        OFString mod_str = module ? " for module '" + OFString(module) + "'" : "";
++
++        OFTEST_LOG_VERBOSE("Running " << tests.size() << " tests" << mod_str << ":");
++
++        for (it = tests.begin(); it != tests.end(); ++it)
++        {
++            OFTEST_LOG_VERBOSE("  Running test '" << (*it)->getTestName() << "'...");
++
++            curTest_ = *it;
++            const OFTestTest::TestResult& result = (*it)->runAndReturn();
++            curTest_ = NULL;
++
++            if (!result.empty())
++            {
++                numFailed++;
++                OFListConstIterator(OFString) rit;
++                for (rit = result.begin(); rit != result.end(); ++rit)
++                {
++                    // recordFailure() already formatted the message
++                    CERR << *rit << OFendl;
++                }
++            }
++        }
++
++        COUT << "Test results" << mod_str << ": "
++            << tests.size() - numFailed << " succeeded, "
++            << numFailed << " failed." << OFendl;
++
++        /* Only the lowest 8 bit of the exit code can be used! */
++        if (numFailed > 254)
++        {
++            CERR << "WARNING: More than 254 tests failed!" << OFendl;
++            return 254;
++        }
++
++        return numFailed;
++    }
++
++    /** Handle the given arguments and run the requested test case. This
++     *  function should be used for implementing main().
++     *  @param argc number of arguments
++     *  @param argv list of arguments
++     *  @param module name of the module that we are testing for
++     *  @return 0 in case of success, else an error value.
++     */
++    int run(int argc, char* argv[], const char* module)
++    {
++        OFList<OFTestTest*> testsToRun;
++        OFBool listOnly = OFFalse;
++
++        OFString rcsid;
++#ifdef OFTEST_OFSTD_ONLY
++        // No proper rcsid because the necessary defines are in dcmdata
++        if (module != NULL)
++            rcsid = "$dcmtk: " + OFString(module) + " $";
++#else
++        rcsid = "$dcmtk: ";
++        rcsid += OFSTRING_GUARD(module);
++        // skip showing version to compile without dcuid.h
++        //        rcsid += " v" OFFIS_DCMTK_VERSION " " OFFIS_DCMTK_RELEASEDATE " $";
++#endif
++
++        OFConsoleApplication app("tests", "Run the test suite", rcsid.c_str());
++        OFCommandLine cmd;
++        cmd.setParamColumn(13);
++
++        cmd.addParam("tests-to-run", "names of tests to run (default: all)", OFCmdParam::PM_MultiOptional);
++
++        cmd.addGroup("general options:");
++          cmd.addOption("--help",       "-h", "print this help text and exit", OFCommandLine::AF_Exclusive);
++          cmd.addOption("--list",       "-l", "list available tests and exit", OFCommandLine::AF_Exclusive);
++          cmd.addOption("--exhaustive", "-x", "also run extensive and slow tests");
++#ifdef OFTEST_OFSTD_ONLY
++          cmd.addOption("--verbose",    "-v", "verbose mode, print processing details");
++#else
++          OFLog::addOptions(cmd);
++#endif
++
++        /* evaluate command line */
++        if (app.parseCommandLine(cmd, argc, argv))
++        {
++            /* check exclusive options first */
++        }
++
++#ifdef OFTEST_OFSTD_ONLY
++        if (cmd.findOption("--verbose")) verbose_ = OFTrue;
++#else
++        /* We disable warnings and errors by default since some tests cause
++         * such messages by testing corner cases. */
++        OFLog::configureFromCommandLine(cmd, app);
++        OFLog::configure(OFLogger::FATAL_LOG_LEVEL);
++#endif
++        if (cmd.findOption("--exhaustive")) exhaustive_ = OFTrue;
++        if (cmd.findOption("--list")) listOnly = OFTrue;
++
++        if (!buildTestsToRun(cmd, testsToRun))
++            return -1;
++
++        if (testsToRun.empty())
++        {
++            CERR << "No tests to run!" << OFendl;
++            return 0;
++        }
++
++        if (listOnly)
++        {
++            OFListIterator(OFTestTest*) it;
++            COUT << "There are " << testsToRun.size() << " tests";
++            if (module)
++                COUT << " for module '" << module << "'";
++            COUT << ":" << OFendl;
++            for (it = testsToRun.begin(); it != testsToRun.end(); ++it)
++            {
++                COUT << "  " << (*it)->getTestName() << "\n";
++            }
++            return 0;
++        }
++
++        return runTests(testsToRun, module);
++    }
++
++private:
++    /// Private constructor, this is a singleton!
++    OFTestManager()
++      : tests_()
++      , curTest_(NULL)
++      , exhaustive_(OFFalse)
++#ifdef OFTEST_OFSTD_ONLY
++      , verbose_(OFFalse)
++#endif
++    {
++    }
++
++    /// Private undefined copy constructor
++    OFTestManager(const OFTestManager& obj);
++
++    /// Private undefined assignment operator
++    OFTestManager& operator=(const OFTestManager& obj);
++
++    /** Build a list of tests which should be executed from the command line.
++     *  @param cmd command line arguments which should be parsed
++     *  @param tests will be set to the list of tests to run
++     *  @return OFFalse if the command line could not be handled.
++     */
++    OFBool buildTestsToRun(OFCommandLine& cmd, OFList<OFTestTest*>& tests) const
++    {
++        const int paramCount = cmd.getParamCount();
++        OFString paramString;
++        OFBool result = OFTrue;
++        if (paramCount == 0)
++        {
++            // If no arguments are given, run all possible tests
++            OFListConstIterator(OFTestTest*) it;
++            for (it = tests_.begin(); it != tests_.end(); ++it)
++            {
++                tests.push_back(*it);
++            }
++        }
++        else
++        {
++            for (int i = 1; i <= paramCount; i++)
++            {
++                cmd.getParam(i, paramString);
++
++                // Find all tests matching this argument
++                OFBool found = OFFalse;
++                OFListConstIterator(OFTestTest*) it;
++                for (it = tests_.begin(); it != tests_.end(); ++it)
++                {
++                    if (testMatches(*it, paramString))
++                    {
++                        tests.push_back(*it);
++                        found = OFTrue;
++                    }
++                }
++
++                if (!found)
++                {
++                    CERR << "Error: No test matches '" << paramString << "'" << OFendl;
++                    result = OFFalse;
++                }
++            }
++        }
++
++        // If we are not in exhaustive mode, remove all slow tests
++        if (!exhaustive_)
++        {
++            OFListIterator(OFTestTest*) it = tests.begin();
++            while (it != tests.end())
++            {
++                if ((*it)->flags() & OFTestTest::EF_Slow)
++                    it = tests.erase(it);
++                else
++                    ++it;
++            }
++        }
++
++        return result;
++    }
++
++    /** Test if the test name matches the given name. This function supports '?'
++     *  and '*' for wildcards. However, '*' can only be used at the end of string.
++     *  @param test test to check against
++     *  @param str the string describing the tests
++     *  @return OFTrue if we found a match, else OFFalse
++     */
++    OFBool testMatches(const OFTestTest* test, const OFString& str) const
++    {
++        const char* testName = test->getTestName().c_str();
++        const char* string = str.c_str();
++
++        for (; *testName != '\0' && *string != '\0'; testName++, string++)
++        {
++            // Does the string still match?
++            if (string[0] != '?' && testName[0] != string[0])
++                break;
++        }
++
++        // Is this a wildcard?
++        // So far we only support '*' at the end of the string
++        if (string[0] == '*' && string[1] == '\0')
++            return OFTrue;
++
++        // If both strings reached their end, we have a match
++        if (testName[0] == '\0' && string[0] == '\0')
++            return OFTrue;
++        return OFFalse;
++    }
++
++    /// List of tests. Statically allocated, so don't have to be freed.
++    OFList<OFTestTest*> tests_;
++
++    /// Currently running test.
++    OFTestTest* curTest_;
++
++    /// Should slow tests be run, too?
++    OFBool exhaustive_;
++
++#ifdef OFTEST_OFSTD_ONLY
++    /// Are we running in verbose mode? Only used if oflog is not available.
++    OFBool verbose_;
++#endif
++};
++
++
++/** Implement a main() function for running tests. The main function will return
++ *  the number of failed tests or -1 if an invalid test name was given.
++ *  @param module the name of the module which is under test.
++ */
++#define OFTEST_MAIN(module) \
++int main(int argc, char* argv[]) \
++{ \
++    return OFTestManager::instance().run(argc, argv, module); \
++}
++
++/// Internal macro for generating a class definition, don't use yourself!
++#define OFTEST_CLASS(testName) \
++class OFTest ## testName : public OFTestTest \
++{ \
++public: \
++    OFTest ## testName(); \
++    void run(); \
++}
++
++/** Register a test to the test manager. Normally you should use
++ *  OFTEST_REGISTER instead, but that macro doesn't work if OFTEST and
++ *  OFTEST_REGISTER are called in the same source file.
++ *  @param testName name of the test to register
++ */
++#define OFTEST_REGISTER_INT(testName) \
++    OFTest ## testName OFTest ## testName ## instance
++
++/** Register a test to the test manager.
++ *  @param testName name of the test to register
++ */
++#define OFTEST_REGISTER(testName) \
++    OFTEST_CLASS(testName); \
++    OFTEST_REGISTER_INT(testName)
++
++/** Macro to define a new test case. Internally this defines a new class
++ *  inheriting from OFTest.
++ *  This is equivalent to OFTEST_FLAGS(testName, EF_None).
++ *  @param testName name describing the test
++ *  @see OFTEST_FLAGS
++ */
++#define OFTEST(testName) OFTEST_FLAGS(testName, EF_None)
++
++/** Macro to define a new test case. Internally this defines a new class
++ *  inheriting from OFTest.
++ *  @param flags flags that should be set for this test
++ *  @param testName name describing the test
++ *  @see OFTEST
++ */
++#define OFTEST_FLAGS(testName, flags) \
++    OFTEST_CLASS(testName); \
++    OFTest ## testName::OFTest ## testName() \
++    : OFTestTest(#testName, flags) \
++    { \
++        OFTestManager::instance().addTest(this); \
++    } \
++    void OFTest ## testName ::run()
++
++/** @name macros for checking conditions in tests
++ *  These macros can be used for doing various checks in test cases. In case
++ *  their check fails, they emit a descriptive message explaining the problem.
++ */
++//@{
++
++/** Check if a condition is true. Can only be used inside OFTEST().
++ *  @param condition condition to check
++ */
++#define OFCHECK(condition) \
++    do { \
++        if (!(condition)) \
++            OFTestManager::instance().currentTest().recordFailure(__FILE__, __LINE__, #condition); \
++    } while (0)
++
++/** Check if two values are equal. Can only be used inside OFTEST(). Both
++ *  arguments must be compatible with OFOStringStream's operator<<.
++ *  @param val1 first value to compare
++ *  @param val2 second value to compare
++ */
++#define OFCHECK_EQUAL(val1, val2) \
++    do { \
++        if ((val1) != (val2)) { \
++            OFOStringStream oss___; \
++            oss___ << "(" << (val1) << ") should equal (" << (val2) << ")" << OFStringStream_ends; \
++            OFSTRINGSTREAM_GETOFSTRING(oss___, str___) \
++            OFTestManager::instance().currentTest().recordFailure(__FILE__, __LINE__, str___); \
++        } \
++    } while (0)
++
++/** Unconditionally add a failure
++ *  @param message string describing the failure
++ */
++#define OFCHECK_FAIL(message) \
++    do { \
++        OFOStringStream oss___; \
++        oss___ << message << OFStringStream_ends; \
++        OFSTRINGSTREAM_GETOFSTRING(oss___, str___) \
++        OFTestManager::instance().currentTest().recordFailure(__FILE__, __LINE__, str___); \
++    } while (0)
++
++//@}
++
++#endif
+diff --git a/ofstd/tests/Makefile.in b/ofstd/tests/Makefile.in
+index aa11e3d..7687db3 100644
+--- a/ofstd/tests/Makefile.in
++++ b/ofstd/tests/Makefile.in
+@@ -12,15 +12,15 @@ configdir = @top_srcdir@/@configdir@
+ 
+ include $(configdir)/@common_makefile@
+ 
+-LOCALINCLUDES = -I$(top_srcdir)/include
++LOCALINCLUDES = -I$(top_srcdir)/include -I$(top_srcdir)/../oflog/include
+ LIBDIRS = -L$(top_srcdir)/libsrc
+ LOCALLIBS = -lofstd
+ LOCALTRASH = *.out
+ 
+ objs = tststack.o tstlist.o tlist.o tstring.o tstthred.o tofstd.o tofdatim.o \
+-       tstatof.o tstftoa.o toffile.o tmap.o tvec.o
++       tstatof.o tstftoa.o toffile.o tmap.o tvec.o taddsub.o tests.o
+ progs = tststack tstlist tlist tstring tstthred tofstd tofdatim tstatof tstftoa \
+-        toffile tmap tvec
++        toffile tmap tvec tests-new-framework
+ 
+ 
+ all: $(progs)
+@@ -61,8 +61,11 @@ tstftoa: tstftoa.o
+ toffile: toffile.o
+ 	$(CXX) $(CXXFLAGS) $(LDFLAGS) $(LIBDIRS) -o $@ $@.o $(LOCALLIBS) $(MATHLIBS) $(LIBS)
+ 
++tests-new-framework: tests.o taddsub.o
++	$(CXX) $(CXXFLAGS) $(LDFLAGS) $(LIBDIRS) -o $@ taddsub.o tests.o $(LOCALLIBS) -L$(top_srcdir)/../oflog/libsrc -loflog $(LIBS)
+ 
+-tests: all test-list test-stack test-string
++
++run-tests: all test-list test-stack test-string test-new-framework-cases
+ 
+ test-list: tlist tstlist
+ 	@echo '--Testing OFList (part 1)'
+@@ -82,6 +85,10 @@ test-string: tstring
+ 	./tstring < tstring.inp > tstring.out
+ 	diff tstring.exp tstring.out
+ 
++test-new-framework-cases: tests-new-framework
++	@echo '--Testing using the new framework'
++	./tests-new-framework > tests-new-framework.out
++	diff tests-new-framework.out tests-new-framework.exp
+ 
+ install: all
+ 
+diff --git a/ofstd/tests/taddsub.cc b/ofstd/tests/taddsub.cc
+new file mode 100644
+index 0000000..41d9160
+--- /dev/null
++++ b/ofstd/tests/taddsub.cc
+@@ -0,0 +1,47 @@
++/*
++ *
++ *  Copyright (C) 2015, Open Connections GmbH
++ *  All rights reserved.  See COPYRIGHT file for details.
++ *
++ *  This software and supporting documentation were developed by
++ *
++ *    OFFIS e.V.
++ *    R&D Division Health
++ *    Escherweg 2
++ *    D-26121 Oldenburg, Germany
++ *
++ *
++ *  Module:  ofstd
++ *
++ *  Author:  Michael Onken
++ *
++ *  Purpose: test program for safe subtraction and addition
++ *
++ */
++
++
++#include "dcmtk/config/osconfig.h"    /* make sure OS specific configuration is included first */
++#include "dcmtk/ofstd/oftest.h"
++#include "dcmtk/ofstd/ofstd.h"
++#include <climits>
++
++
++OFTEST(ofstd_safeSubtractAndAdd)
++{
++  // --------------- Subtraction ----------------
++
++  unsigned int a = 1;
++  unsigned int b = 2;
++  // check whether underflow occurs (it should)
++  OFCHECK(OFStandard::safeSubtract(a, b, a) == OFFalse);
++  // check whether a has not been modified
++  OFCHECK_EQUAL(a, 1);
++
++  a = UINT_MAX;
++  b = UINT_MAX-1;
++  // check whether no underflow occured (it shouldnt)
++  OFCHECK(OFStandard::safeSubtract(a, b, a) == OFTrue);
++  // check whether the result a was computed as expected
++  OFCHECK_EQUAL(a, 1);
++
++}
+diff --git a/ofstd/tests/tests-new-framework.exp b/ofstd/tests/tests-new-framework.exp
+new file mode 100644
+index 0000000..63f66ee
+--- /dev/null
++++ b/ofstd/tests/tests-new-framework.exp
+@@ -0,0 +1 @@
++Test results for module 'ofstd': 1 succeeded, 0 failed.
+diff --git a/ofstd/tests/tests.cc b/ofstd/tests/tests.cc
+new file mode 100644
+index 0000000..414cf76
+--- /dev/null
++++ b/ofstd/tests/tests.cc
+@@ -0,0 +1,28 @@
++/*
++ *
++ *  Copyright (C) 2011-2014, OFFIS e.V.
++ *  All rights reserved.  See COPYRIGHT file for details.
++ *
++ *  This software and supporting documentation were developed by
++ *
++ *    OFFIS e.V.
++ *    R&D Division Health
++ *    Escherweg 2
++ *    D-26121 Oldenburg, Germany
++ *
++ *
++ *  Module:  ofstd
++ *
++ *  Author:  Uli Schlachter
++ *
++ *  Purpose: main test program
++ *
++ */
++
++#include "dcmtk/config/osconfig.h"
++
++#define OFTEST_OFSTD_ONLY
++#include "dcmtk/ofstd/oftest.h"
++
++OFTEST_REGISTER(ofstd_safeSubtractAndAdd);
++OFTEST_MAIN("ofstd")
+-- 
+2.1.4
+
diff -Nru dcmtk-3.6.0/debian/patches/series dcmtk-3.6.0/debian/patches/series
--- dcmtk-3.6.0/debian/patches/series	2012-05-31 10:11:33.000000000 +0200
+++ dcmtk-3.6.0/debian/patches/series	2016-12-19 20:49:56.000000000 +0100
@@ -11,3 +11,4 @@
 removecharls.patch
 bug674361.patch
 underlink.patch
+0001-Fixed-possible-underflows-and-overflows.patch
diff -Nru dcmtk-3.6.0/debian/rules dcmtk-3.6.0/debian/rules
--- dcmtk-3.6.0/debian/rules	2012-12-20 13:22:26.000000000 +0100
+++ dcmtk-3.6.0/debian/rules	2016-12-20 03:27:35.000000000 +0100
@@ -91,6 +91,13 @@
 	$(MAKE) clean && \
 	$(MAKE)
 
+	# Run tests covering the fix for CVE-2015-8979.
+	# The tests executables are searching for *.so.2 which are not available yet,
+	# but the LD_PRELOAD trick loads the right libraries.
+	(export LD_LIBRARY_PATH=$(CURDIR)/oflog/libsrc:$(CURDIR)/ofstd/libsrc; \
+		export LD_PRELOAD=libofstd.so:liboflog.so; \
+		$(MAKE) -C ofstd/tests run-tests)
+
 	touch build-arch-stamp
 
 build-indep: config/config.status  build-indep-stamp

Reply to: