changeset 40812:9cdd525d97b2 stable

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.
author Yuya Nishihara <yuya@tcha.org>
date Thu, 01 Nov 2018 20:32:59 +0900
parents 44c2e80db985
children 884321cd26c3
files mercurial/cext/revlog.c tests/test-parseindex.t
diffstat 2 files changed, 26 insertions(+), 3 deletions(-) [+]
line wrap: on
line diff
--- 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;
 	}
--- 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