Fix infinite loop trimming passwords with ( in them

Jay Berkenbilt
1 parent 8f5de08c
Showing 3 changed files with 28 additions and 4 deletions
libqpdf/QPDF_encryption.cc
qpdf/qpdf.testcov
qpdf/test_driver.cc
@@ -141,14 +141,20 @@ QPDF::trim_user_password(std::string&amp; user_password)
 	return;
     }
-    char const* p = 0;
-    while ((p = strchr(cstr, '\x28')) != 0)
+    char const* p1 = cstr;
+    char const* p2 = 0;
+    while ((p2 = strchr(p1, '\x28')) != 0)
     {
-	if (memcmp(p, padding_string, len - (p - cstr)) == 0)
+	if (memcmp(p2, padding_string, len - (p2 - cstr)) == 0)
 	{
-	    user_password = user_password.substr(0, p - cstr);
+	    user_password = user_password.substr(0, p2 - cstr);
 	    return;
 	}
+        else
+        {
+            QTC::TC("qpdf", "QPDF_encryption skip 0x28");
+            p1 = p2 + 1;
+        }
     }
 }
@@ -253,3 +253,4 @@ QPDFWriter skip ADBE 0
 QPDFWriter remove existing Extensions 0
 QPDFWriter skip Extensions 0
 QPDFWriter preserve ADBE 0
+QPDF_encryption skip 0x28 0
@@ -88,6 +88,23 @@ void runtest(int n, char const* filename1, char const* filename2)
     // the test suite to see how the test is invoked to find the file
     // that the test is supposed to operate on.
+    if (n == 0)
+    {
+        // Throw in some random test cases that don't fit anywhere
+        // else.  This is in addition to whatever else is going on in
+        // test 0.
+
+        // The code to trim user passwords looks for 0x28 (which is
+        // "(") since it marks the beginning of the padding.  Exercise
+        // the code to make sure it skips over 0x28 characters that
+        // aren't part of padding.
+        std::string password(
+            "1234567890123456789012(45678\x28\xbf\x4e\x5e");
+        assert(password.length() == 32);
+        QPDF::trim_user_password(password);
+        assert(password == "1234567890123456789012(45678");
+    }
+
     QPDF pdf;
     PointerHolder<char> file_buf;
     FILE* filep = 0;