Hi Moritz,

On  Di 06 Nov 2018 17:14:35 CET, Moritz Mühlenhoff wrote:

On Fri, Sep 28, 2018 at 08:32:25PM +0200, Markus Koschany wrote:
Package: poppler
X-Debbugs-CC: t...@security.debian.org
Severity: important
Tags: security

Hi,

The following vulnerability was published for poppler.

CVE-2018-16646[0]:
| In Poppler 0.68.0, the Parser::getObj() function in Parser.cc may cause
| infinite recursion via a crafted file. A remote attacker can leverage
| this for a DoS attack.

For jessie the wrong patches got applied. They are based on MR 67, which
didn't get merged in favour of the patch from MR 91.

On a more general notice: This bug has virtually no security impact, it's
hard too see why this change was made for an LTS release to begin with,
but at least wait until it's applied/fixed in unstable before backporting.

Not security, but functionality. With the proof of malign content, I could successfully freeze 1 core on the test system endlessly.

Unideal is the application of the wrong MR (for others, see 17c991f7992270d9f7ecf004741c1c3acc235b8d in security-tracker).

I have a modified version (+deb8u6, regression fix) ready for upload to jessie LTS (see attached .debdiff). @Moritz: do you see any reason for holding it back at this moment?

Thanks+Greets,
Mike


--

DAS-NETZWERKTEAM
mike gabriel, herweg 7, 24357 fleckeby
mobile: +49 (1520) 1976 148
landline: +49 (4354) 8390 139

GnuPG Fingerprint: 9BFB AEE8 6C0A A5FF BF22  0782 9AF4 6B30 2577 1B31
mail: mike.gabr...@das-netzwerkteam.de, http://das-netzwerkteam.de

diff -Nru poppler-0.26.5/debian/changelog poppler-0.26.5/debian/changelog
--- poppler-0.26.5/debian/changelog     2018-10-31 12:47:07.000000000 +0100
+++ poppler-0.26.5/debian/changelog     2018-11-08 11:38:57.000000000 +0100
@@ -1,3 +1,11 @@
+poppler (0.26.5-2+deb8u6) jessie-security; urgency=medium
+
+  * CVE-2018-16646: Regression fix. Apply the patch from
+    upstream's MR #91 (which got merged upstream) and not
+    the proposed patches from MR #67.
+
+ -- Mike Gabriel <sunwea...@debian.org>  Thu, 08 Nov 2018 11:38:57 +0100
+
 poppler (0.26.5-2+deb8u5) jessie-security; urgency=medium
 
   * Non-maintainer upload by the LTS Team.
diff -Nru poppler-0.26.5/debian/patches/mupstream_CVE-2018-16646.patch 
poppler-0.26.5/debian/patches/mupstream_CVE-2018-16646.patch
--- poppler-0.26.5/debian/patches/mupstream_CVE-2018-16646.patch        
1970-01-01 01:00:00.000000000 +0100
+++ poppler-0.26.5/debian/patches/mupstream_CVE-2018-16646.patch        
2018-11-08 11:26:48.000000000 +0100
@@ -0,0 +1,62 @@
+From e16f2d5bf39842c647d413bbd6e16de73c76a2c8 Mon Sep 17 00:00:00 2001
+From: Marek Kasik <mka...@redhat.com>
+Date: Mon, 22 Oct 2018 16:56:01 +0200
+Subject: [PATCH] Avoid cycles in PDF parsing
+
+Mark objects being processed in Parser::makeStream() as being processed
+and check the mark when entering this method to avoid processing
+of the same object recursively.
+---
+ poppler/Parser.cc | 15 +++++++++++++++
+ poppler/XRef.h    |  1 +
+ 2 files changed, 16 insertions(+)
+
+diff --git a/poppler/Parser.cc b/poppler/Parser.cc
+index bd4845ab..6c8ae703 100644
+--- a/poppler/Parser.cc
++++ b/poppler/Parser.cc
+@@ -197,6 +197,18 @@ Stream *Parser::makeStream(Object &&dict, Guchar *fileKey,
+   Stream *str;
+   Goffset length;
+   Goffset pos, endPos;
++  XRefEntry *entry;
++
++  if (xref && (entry = xref->getEntry(objNum, gFalse))) {
++    if (!entry->getFlag(XRefEntry::Parsing) ||
++        (objNum == 0 && objGen == 0)) {
++      entry->setFlag(XRefEntry::Parsing, gTrue);
++    } else {
++      error(errSyntaxError, getPos(),
++            "Object '{0:d} {1:d} obj' is being already parsed", objNum, 
objGen);
++      return nullptr;
++    }
++  }
+ 
+   // get stream start position
+   lexer->skipToNextLine();
+@@ -278,6 +290,9 @@ Stream *Parser::makeStream(Object &&dict, Guchar *fileKey,
+   // get filters
+   str = str->addFilters(str->getDict(), recursion);
+ 
++  if (entry)
++    entry->setFlag(XRefEntry::Parsing, gFalse);
++
+   return str;
+ }
+ 
+diff --git a/poppler/XRef.h b/poppler/XRef.h
+index 11ee5e03..2eb2f9fd 100644
+--- a/poppler/XRef.h
++++ b/poppler/XRef.h
+@@ -65,6 +65,7 @@ struct XRefEntry {
+   enum Flag {
+     // Regular flags
+     Updated,     // Entry was modified
++    Parsing,     // Entry is currently being parsed
+ 
+     // Special flags -- available only after xref->scanSpecialFlags() is run
+     Unencrypted, // Entry is stored in unencrypted form (meaningless in 
unencrypted documents)
+-- 
+2.18.1
+
+
diff -Nru poppler-0.26.5/debian/patches/series 
poppler-0.26.5/debian/patches/series
--- poppler-0.26.5/debian/patches/series        2018-10-31 12:47:07.000000000 
+0100
+++ poppler-0.26.5/debian/patches/series        2018-11-08 11:29:46.000000000 
+0100
@@ -20,8 +20,7 @@
 upstream_Fix-rendering-of-some-broken-PDF-files.patch
 upstream_Allow-newlines-in-num-gen-obj-sequence.patch
 upstream_XRef-Fix-runtime-undefined-behaviour.patch
-upstream_CVE-2018-16646_Fail-when-PDF-contains-duplicate-objects.patch
-upstream_CVE-2018-16646_Allow-duplicated-objects-in-incremental-updates.patch
+upstream_CVE-2018-16646.patch
 upstream_CVE-2018-10768.patch
 upstream_CVE-2017-18267.patch
 upstream-modified_CVE-2018-13988.patch
diff -Nru 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646_Allow-duplicated-objects-in-incremental-updates.patch
 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646_Allow-duplicated-objects-in-incremental-updates.patch
--- 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646_Allow-duplicated-objects-in-incremental-updates.patch
 2018-10-30 21:19:17.000000000 +0100
+++ 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646_Allow-duplicated-objects-in-incremental-updates.patch
 1970-01-01 01:00:00.000000000 +0100
@@ -1,75 +0,0 @@
-From a1044e6639316ee876031595a0c09889c28098ce Mon Sep 17 00:00:00 2001
-From: Marek Kasik <mka...@redhat.com>
-Date: Mon, 15 Oct 2018 17:53:45 +0200
-Subject: [PATCH 2/2] Allow duplicated objects in incremental updates
-
-Do not fail when encountering duplicated object in different
-update section of the file.
----
- poppler/XRef.cc | 18 ++++++++++++++----
- 1 file changed, 14 insertions(+), 4 deletions(-)
-
---- a/poppler/XRef.cc
-+++ b/poppler/XRef.cc
-@@ -905,6 +905,7 @@
-   char* token = NULL;
-   bool oneCycle = true;
-   int offset = 0;
-+  std::vector<bool> seen;
- 
-   gfree(entries);
-   capacity = 0;
-@@ -925,6 +926,9 @@
-     if (!str->getLine(buf, 256)) {
-       break;
-     }
-+    if (strstr(buf, "%%EOF")) {
-+      seen.assign(seen.size(), false);
-+    }
-     p = buf;
- 
-     // skip whitespace
-@@ -977,6 +981,9 @@
-           if ((*p & 0xff) == 0) {
-             //new line, continue with next line!
-             str->getLine(buf, 256);
-+            if (strstr(buf, "%%EOF")) {
-+              seen.assign(seen.size(), false);
-+            }
-             p = buf;
-           } else {
-           ++p;
-@@ -991,6 +998,9 @@
-               if ((*p & 0xff) == 0) {
-                 //new line, continue with next line!
-                 str->getLine(buf, 256);
-+                if (strstr(buf, "%%EOF")) {
-+                  seen.assign(seen.size(), false);
-+                }
-                 p = buf;
-               } else {
-               ++p;
-@@ -1006,12 +1016,11 @@
-                   if (resize(newSize) != newSize) {
-                     error(errSyntaxError, -1, "Invalid 'obj' parameters");
-                     return gFalse;
-+                  } else {
-+                    seen.resize (newSize, false);
-                   }
-                 }
--                if (entries[num].type != xrefEntryFree &&
--                    gen == entries[num].gen &&
--                    entries[num].offset != pos - start &&
--                    entries[num].offset != -1) {
-+                if (seen[num]) {
-                   error(errSyntaxError, -1, "Duplicate object reference {0:d} 
{1:d}", num, gen);
-                   return gFalse;
-                 }
-@@ -1020,6 +1029,7 @@
-                   entries[num].offset = pos - start;
-                   entries[num].gen = gen;
-                   entries[num].type = xrefEntryUncompressed;
-+                  seen[num] = true;
-               }
-               }
-             }
diff -Nru 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646_Fail-when-PDF-contains-duplicate-objects.patch
 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646_Fail-when-PDF-contains-duplicate-objects.patch
--- 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646_Fail-when-PDF-contains-duplicate-objects.patch
        2018-10-30 21:13:35.000000000 +0100
+++ 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646_Fail-when-PDF-contains-duplicate-objects.patch
        1970-01-01 01:00:00.000000000 +0100
@@ -1,29 +0,0 @@
-From 9ea471459f37b4595f3032e64d2c6a659c050529 Mon Sep 17 00:00:00 2001
-From: Marek Kasik <mka...@redhat.com>
-Date: Wed, 26 Sep 2018 16:13:06 +0200
-Subject: [PATCH 1/2] Fail when PDF contains duplicate objects
-
-Check whether current xrefEntry is already occupied during 
XRef::constructXRef().
-If yes then check its offset against current position and fail if they
-are different.
-This helps with files which reach recursionLimit in Parser.cc.
----
- poppler/XRef.cc | 7 +++++++
- 1 file changed, 7 insertions(+)
-
---- a/poppler/XRef.cc
-+++ b/poppler/XRef.cc
-@@ -1008,6 +1008,13 @@
-                     return gFalse;
-                   }
-                 }
-+                if (entries[num].type != xrefEntryFree &&
-+                    gen == entries[num].gen &&
-+                    entries[num].offset != pos - start &&
-+                    entries[num].offset != -1) {
-+                  error(errSyntaxError, -1, "Duplicate object reference {0:d} 
{1:d}", num, gen);
-+                  return gFalse;
-+                }
-                 if (entries[num].type == xrefEntryFree ||
-                     gen >= entries[num].gen) {
-                   entries[num].offset = pos - start;
diff -Nru poppler-0.26.5/debian/patches/upstream_CVE-2018-16646.patch 
poppler-0.26.5/debian/patches/upstream_CVE-2018-16646.patch
--- poppler-0.26.5/debian/patches/upstream_CVE-2018-16646.patch 1970-01-01 
01:00:00.000000000 +0100
+++ poppler-0.26.5/debian/patches/upstream_CVE-2018-16646.patch 2018-11-08 
11:29:13.000000000 +0100
@@ -0,0 +1,54 @@
+From e16f2d5bf39842c647d413bbd6e16de73c76a2c8 Mon Sep 17 00:00:00 2001
+From: Marek Kasik <mka...@redhat.com>
+Date: Mon, 22 Oct 2018 16:56:01 +0200
+Subject: [PATCH] Avoid cycles in PDF parsing
+
+Mark objects being processed in Parser::makeStream() as being processed
+and check the mark when entering this method to avoid processing
+of the same object recursively.
+---
+ poppler/Parser.cc | 15 +++++++++++++++
+ poppler/XRef.h    |  1 +
+ 2 files changed, 16 insertions(+)
+
+--- a/poppler/Parser.cc
++++ b/poppler/Parser.cc
+@@ -197,6 +197,18 @@
+   Stream *str;
+   Goffset length;
+   Goffset pos, endPos;
++  XRefEntry *entry;
++
++  if (xref && (entry = xref->getEntry(objNum, gFalse))) {
++    if (!entry->getFlag(XRefEntry::Parsing) ||
++        (objNum == 0 && objGen == 0)) {
++      entry->setFlag(XRefEntry::Parsing, gTrue);
++    } else {
++      error(errSyntaxError, getPos(),
++            "Object '{0:d} {1:d} obj' is being already parsed", objNum, 
objGen);
++      return nullptr;
++    }
++  }
+ 
+   // get stream start position
+   lexer->skipToNextLine();
+@@ -276,6 +288,9 @@
+   // get filters
+   str = str->addFilters(dict, recursion);
+ 
++  if (entry)
++    entry->setFlag(XRefEntry::Parsing, gFalse);
++
+   return str;
+ }
+ 
+--- a/poppler/XRef.h
++++ b/poppler/XRef.h
+@@ -69,6 +69,7 @@
+   enum Flag {
+     // Regular flags
+     Updated,     // Entry was modified
++    Parsing,     // Entry is currently being parsed
+ 
+     // Special flags -- available only after xref->scanSpecialFlags() is run
+     Unencrypted, // Entry is stored in unencrypted form (meaningless in 
unencrypted documents)

Attachment: pgph6z7AODQvM.pgp
Description: Digitale PGP-Signatur

Reply via email to