# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1541071979 -32400
# Node ID 9cdd525d97b24812f525056d831ce456c8ba714a
# Parent  44c2e80db98532eea9ee3f3ec226e199e1e8a72e
revlog: fix out-of-bounds access by negative parents read from revlog (SEC)

82d6a35cf432 wasn't enough. Several callers don't check negative revisions
but for -1 (nullrev), which would directly lead to out-of-bounds read, and
buffer overflow could follow. RCE might be doable with carefully crafted
revlog structure, though I don't think this would be useful attack surface.

diff -r 44c2e80db985 -r 9cdd525d97b2 mercurial/cext/revlog.c
--- a/mercurial/cext/revlog.c	Mon Dec 03 11:14:44 2018 -0800
+++ b/mercurial/cext/revlog.c	Thu Nov 01 20:32:59 2018 +0900
@@ -157,6 +157,12 @@
 	return (const char *)(self->buf.buf) + pos * v1_hdrsize;
 }
 
+/*
+ * Get parents of the given rev.
+ *
+ * The specified rev must be valid and must not be nullrev. A returned
+ * parent revision may be nullrev, but is guaranteed to be in valid range.
+ */
 static inline int index_get_parents(indexObject *self, Py_ssize_t rev,
 				    int *ps, int maxrev)
 {
@@ -171,7 +177,7 @@
 	}
 	/* If index file is corrupted, ps[] may point to invalid revisions. So
 	 * there is a risk of buffer overflow to trust them unconditionally. */
-	if (ps[0] > maxrev || ps[1] > maxrev) {
+	if (ps[0] < -1 || ps[0] > maxrev || ps[1] < -1 || ps[1] > maxrev) {
 		PyErr_SetString(PyExc_ValueError, "parent out of range");
 		return -1;
 	}
diff -r 44c2e80db985 -r 9cdd525d97b2 tests/test-parseindex.t
--- a/tests/test-parseindex.t	Mon Dec 03 11:14:44 2018 -0800
+++ b/tests/test-parseindex.t	Thu Nov 01 20:32:59 2018 +0900
@@ -133,12 +133,18 @@
   $ cd invalidparent
 
   $ hg clone --pull -q --config phases.publish=False ../a limit
+  $ hg clone --pull -q --config phases.publish=False ../a neglimit
   $ hg clone --pull -q --config phases.publish=False ../a segv
-  $ rm -R limit/.hg/cache segv/.hg/cache
+  $ rm -R limit/.hg/cache neglimit/.hg/cache segv/.hg/cache
 
   $ "$PYTHON" <<EOF
   > data = open("limit/.hg/store/00changelog.i", "rb").read()
-  > for n, p in [(b'limit', b'\0\0\0\x02'), (b'segv', b'\0\x01\0\0')]:
+  > poisons = [
+  >     (b'limit', b'\0\0\0\x02'),
+  >     (b'neglimit', b'\xff\xff\xff\xfe'),
+  >     (b'segv', b'\0\x01\0\0'),
+  > ]
+  > for n, p in poisons:
   >     # corrupt p1 at rev0 and p2 at rev1
   >     d = data[:24] + p + data[28:127 + 28] + p + data[127 + 32:]
   >     open(n + b"/.hg/store/00changelog.i", "wb").write(d)
@@ -154,6 +160,11 @@
         0       1        1       -1    base         63         62         63   1.01613        63         0    0.00000
         1       2        1       -1    base         66         65         66   1.01538        66         0    0.00000
 
+  $ hg -R neglimit debugrevlogindex -f1 -c
+     rev flag     size   link     p1     p2       nodeid
+       0 0000       62      0     -2     -1 7c31755bf9b5
+       1 0000       65      1      0     -2 26333235a41c
+
   $ hg -R segv debugrevlogindex -f1 -c
      rev flag     size   link     p1     p2       nodeid
        0 0000       62      0  65536     -1 7c31755bf9b5
@@ -193,6 +204,12 @@
   index_headrevs: parent out of range
   find_gca_candidates: parent out of range
   find_deepest: parent out of range
+  $ "$PYTHON" test.py neglimit/.hg/store
+  reachableroots: parent out of range
+  compute_phases_map_sets: parent out of range
+  index_headrevs: parent out of range
+  find_gca_candidates: parent out of range
+  find_deepest: parent out of range
   $ "$PYTHON" test.py segv/.hg/store
   reachableroots: parent out of range
   compute_phases_map_sets: parent out of range