D5256: manifest: make sure there's a filename before bothering to look for newline

[durin42 (Augie Fackler)]

durin42 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  There's no valid manifest that would have no characters before the NUL byte on
  a line, and this fixes some erratic timeouts in the fuzzer.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5256

AFFECTED FILES
  mercurial/cext/manifest.c
  tests/test-manifest.py

CHANGE DETAILS

diff --git a/tests/test-manifest.py b/tests/test-manifest.py
--- a/tests/test-manifest.py
+++ b/tests/test-manifest.py
@@ -4,6 +4,7 @@
 import itertools
 import silenttestrunner
 import unittest
+import zlib
 
 from mercurial import (
     manifest as manifestmod,
@@ -397,6 +398,29 @@
     def parsemanifest(self, text):
         return manifestmod.manifestdict(text)
 
+    def testObviouslyBogusManifest(self):
+        # This is a 163k manifest that came from oss-fuzz. It was a
+        # timeout there, but when run normally it doesn't seem to
+        # present any particular slowness.
+        data = zlib.decompress(
+            'x\x9c\xed\xce;\n\x83\x00\x10\x04\xd0\x8deNa\x93~\xf1\x03\xc9q\xf4'
+            '\x14\xeaU\xbdB\xda\xd4\xe6Cj\xc1FA\xde+\x86\xe9f\xa2\xfci\xbb\xfb'
+            '\xa3\xef\xea\xba\xca\x7fk\x86q\x9a\xc6\xc8\xcc&\xb3\xcf\xf8\xb8|#'
+            '\x8a9\x00\xd8\xe6v\xf4\x01N\xe1\n\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
+            '\x00\x00\xc0\x8aey\x1d}\x01\xd8\xe0\xb9\xf3\xde\x1b\xcf\x17'
+            '\xac\xbe')
+        with self.assertRaises(ValueError):
+            m = self.parsemanifest(data)
+
 class testtreemanifest(unittest.TestCase, basemanifesttests):
     def parsemanifest(self, text):
         return manifestmod.treemanifest(b'', text)
diff --git a/mercurial/cext/manifest.c b/mercurial/cext/manifest.c
--- a/mercurial/cext/manifest.c
+++ b/mercurial/cext/manifest.c
@@ -38,6 +38,7 @@
 #define MANIFEST_OOM -1
 #define MANIFEST_NOT_SORTED -2
 #define MANIFEST_MALFORMED -3
+#define MANIFEST_BOGUS_FILENAME -4
 
 /* get the length of the path for a line */
 static size_t pathlen(line *l)
@@ -115,7 +116,13 @@
 	char *prev = NULL;
 	while (len > 0) {
 		line *l;
-		char *next = memchr(data, '\n', len);
+		char *next;
+		if (*data == '\0') {
+			/* It's implausible there's no filename, don't
+			 * even bother looking for the newline. */
+			return MANIFEST_BOGUS_FILENAME;
+		}
+		next = memchr(data, '\n', len);
 		if (!next) {
 			return MANIFEST_MALFORMED;
 		}
@@ -190,6 +197,11 @@
 		PyErr_Format(PyExc_ValueError,
 			     "Manifest did not end in a newline.");
 		break;
+	case MANIFEST_BOGUS_FILENAME:
+		PyErr_Format(
+			PyExc_ValueError,
+			"Manifest had an entry with a zero-length filename.");
+		break;
 	default:
 		PyErr_Format(PyExc_ValueError,
 			     "Unknown problem parsing manifest.");



To: durin42, #hg-reviewers
Cc: mercurial-devel