CVE-2021-36978: Out-of-bounds Write (CWE-787)
value: QPDF 9.x through 9.1.1 and 10.x through 10.0.4 has a heap-based buffer overflow in Pl_ASCII85Decoder::write (called from Pl_AES_PDF::flush and Pl_AES_PDF::finish) when a certain downstream write fails.
CWE description: The product writes data past the end, or before the beginning, of the intended buffer.
CVSS v3 base score: 5.5
Published: 2021-07-20T07:15Z
Repository: https://github.com/qpdf/qpdf
Language: C++
Files changed: ChangeLog, libqpdf/Pl_AES_PDF.cc, libqpdf/Pl_ASCII85Decoder.cc, libqpdf/Pl_ASCIIHexDecoder.cc, libqpdf/Pl_Count.cc
Commit date: 2021-01-04 15:17:35 -0500
Commit message: Fix some pipelines to be safe if downstream write fails (fuzz issue 28262)

Diff: diff --git a/ChangeLog b/ChangeLog
index 2db53cb8..1e31efb4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
+2021-01-04  Jay Berkenbilt  <ejb@ql.org>
+
+	* Move getNext()->write() calls in some pipelines to ensure that
+	state gates properly reset even if the next pipeline's write
+	throws an exception (fuzz issue 28262).
+
 2021-01-03  Jay Berkenbilt  <ejb@ql.org>
 
 	* Don't include -o nospace with zsh completion setup so file
 	completion works normally. Fixes #473.
 
diff --git a/fuzz/qpdf_extra/28262.fuzz b/fuzz/qpdf_extra/28262.fuzz
new file mode 100644
index 00000000..4e872ba4
Binary files /dev/null and b/fuzz/qpdf_extra/28262.fuzz differ
diff --git a/libqpdf/Pl_AES_PDF.cc b/libqpdf/Pl_AES_PDF.cc
index 18cf3a4d..2865f804 100644
--- a/libqpdf/Pl_AES_PDF.cc
+++ b/libqpdf/Pl_AES_PDF.cc
@@ -236,8 +236,8 @@ Pl_AES_PDF::flush(bool strip_padding)
 	    {
 		bytes -= last;
 	    }
 	}
     }
-    getNext()->write(this->outbuf, bytes);
     this->offset = 0;
+    getNext()->write(this->outbuf, bytes);
 }
diff --git a/libqpdf/Pl_ASCII85Decoder.cc b/libqpdf/Pl_ASCII85Decoder.cc
index b8df3e87..9d9f6704 100644
--- a/libqpdf/Pl_ASCII85Decoder.cc
+++ b/libqpdf/Pl_ASCII85Decoder.cc
@@ -117,14 +117,17 @@ Pl_ASCII85Decoder::flush()
 	lval >>= 8;
     }
 
     QTC::TC("libtests", "Pl_ASCII85Decoder partial flush",
 	    (this->pos == 5) ? 0 : 1);
-    getNext()->write(outbuf, this->pos - 1);
-
+    // Reset before calling getNext()->write in case that throws an
+    // exception.
+    auto t = this->pos - 1;
     this->pos = 0;
     memset(this->inbuf, 117, 5);
+
+    getNext()->write(outbuf, t);
 }
 
 void
 Pl_ASCII85Decoder::finish()
 {
diff --git a/libqpdf/Pl_ASCIIHexDecoder.cc b/libqpdf/Pl_ASCIIHexDecoder.cc
index f20a9769..7845268e 100644
--- a/libqpdf/Pl_ASCIIHexDecoder.cc
+++ b/libqpdf/Pl_ASCIIHexDecoder.cc
@@ -95,16 +95,18 @@ Pl_ASCIIHexDecoder::flush()
     }
     unsigned char ch = static_cast<unsigned char>((b[0] << 4) + b[1]);
 
     QTC::TC("libtests", "Pl_ASCIIHexDecoder partial flush",
 	    (this->pos == 2) ? 0 : 1);
-    getNext()->write(&ch, 1);
-
+    // Reset before calling getNext()->write in case that throws an
+    // exception.
     this->pos = 0;
     this->inbuf[0] = '0';
     this->inbuf[1] = '0';
     this->inbuf[2] = '\0';
+
+    getNext()->write(&ch, 1);
 }
 
 void
 Pl_ASCIIHexDecoder::finish()
 {
diff --git a/libqpdf/Pl_Count.cc b/libqpdf/Pl_Count.cc
index 8077092a..c35619b8 100644
--- a/libqpdf/Pl_Count.cc
+++ b/libqpdf/Pl_Count.cc
@@ -25,12 +25,12 @@ void
 Pl_Count::write(unsigned char* buf, size_t len)
 {
     if (len)
     {
 	this->m->count += QIntC::to_offset(len);
-	getNext()->write(buf, len);
 	this->m->last_char = buf[len - 1];
+	getNext()->write(buf, len);
     }
 }
 
 void
 Pl_Count::finish()

Vulnerable code:     getNext()->write(this->outbuf, bytes);
    getNext()->write(outbuf, this->pos - 1);

    getNext()->write(&ch, 1);

	getNext()->write(buf, len);
Fixed code: 2021-01-04  Jay Berkenbilt  <ejb@ql.org>

	* Move getNext()->write() calls in some pipelines to ensure that
	state gates properly reset even if the next pipeline's write
	throws an exception (fuzz issue 28262).

    getNext()->write(this->outbuf, bytes);
    // Reset before calling getNext()->write in case that throws an
    // exception.
    auto t = this->pos - 1;

    getNext()->write(outbuf, t);
    // Reset before calling getNext()->write in case that throws an
    // exception.

    getNext()->write(&ch, 1);
	getNext()->write(buf, len);