revlog: fix out-of-bounds access by negative parents read from revlog (SEC) stable
authorYuya Nishihara <yuya@tcha.org>
Thu, 01 Nov 2018 20:32:59 +0900
branchstable
changeset 53578 9cdd525d97b24812f525056d831ce456c8ba714a
parent 53560 44c2e80db98532eea9ee3f3ec226e199e1e8a72e
child 53579 884321cd26c30d4ea7a693b467c41270e612cde8
push id1079
push usergszorc@mozilla.com
push dateMon, 10 Dec 2018 19:44:59 +0000
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.
mercurial/cext/revlog.c
tests/test-parseindex.t
--- a/mercurial/cext/revlog.c
+++ b/mercurial/cext/revlog.c
@@ -152,31 +152,37 @@ static const char *index_deref(indexObje
 			inline_scan(self, self->offsets);
 		}
 		return self->offsets[pos];
 	}
 
 	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)
 {
 	if (rev >= self->length) {
 		PyObject *tuple = PyList_GET_ITEM(self->added, rev - self->length);
 		ps[0] = (int)PyInt_AS_LONG(PyTuple_GET_ITEM(tuple, 5));
 		ps[1] = (int)PyInt_AS_LONG(PyTuple_GET_ITEM(tuple, 6));
 	} else {
 		const char *data = index_deref(self, rev);
 		ps[0] = getbe32(data + 24);
 		ps[1] = getbe32(data + 28);
 	}
 	/* 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;
 	}
 	return 0;
 }
 
 
 /*
--- a/tests/test-parseindex.t
+++ b/tests/test-parseindex.t
@@ -128,37 +128,48 @@ Test SEGV caused by bad revision passed 
   $ cd ..
 
 Test corrupted p1/p2 fields that could cause SEGV at parsers.c:
 
   $ mkdir invalidparent
   $ 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)
   > EOF
 
   $ hg -R limit 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 limit debugdeltachain -c
       rev  chain# chainlen     prev   delta       size    rawsize  chainsize     ratio   lindist extradist extraratio
         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
        1 0000       65      1      0  65536 26333235a41c
 
   $ hg -R segv debugdeltachain -c
       rev  chain# chainlen     prev   delta       size    rawsize  chainsize     ratio   lindist extradist extraratio
         0       1        1       -1    base         63         62         63   1.01613        63         0    0.00000
@@ -188,16 +199,22 @@ Test corrupted p1/p2 fields that could c
   > EOF
 
   $ "$PYTHON" test.py limit/.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 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
   index_headrevs: parent out of range
   find_gca_candidates: parent out of range
   find_deepest: parent out of range
 
   $ cd ..