Patchwork [STABLE] parsers: fix segfault by invalid parent revision read from revlog

login
register
mail settings
Submitter Yuya Nishihara
Date July 16, 2015, 4:20 p.m.
Message ID <6249ca2cf0a01c9325bc.1437063612@mimosa>
Download mbox | patch
Permalink /patch/10010/
State Accepted
Headers show

Comments

Yuya Nishihara - July 16, 2015, 4:20 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1437057368 -32400
#      Thu Jul 16 23:36:08 2015 +0900
# Branch stable
# Node ID 6249ca2cf0a01c9325bc3537822879df4240d586
# Parent  501c51d607922692bd6654cb3c24d9f1e31d7450
parsers: fix segfault by invalid parent revision read from revlog

If revlog file is corrupted, it can have parent pointing to invalid revision.
So we should validate it before updating nothead[] and phases[]. Otherwise it
would segfault at best.
Matt Mackall - July 16, 2015, 5:45 p.m.
On Fri, 2015-07-17 at 01:20 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1437057368 -32400
> #      Thu Jul 16 23:36:08 2015 +0900
> # Branch stable
> # Node ID 6249ca2cf0a01c9325bc3537822879df4240d586
> # Parent  501c51d607922692bd6654cb3c24d9f1e31d7450
> parsers: fix segfault by invalid parent revision read from revlog
> 
> If revlog file is corrupted, it can have parent pointing to invalid revision.
> So we should validate it before updating nothead[] and phases[]. Otherwise it
> would segfault at best.

This looks ok, but conflicts with default. I'd rather just apply a patch
to default since the -rc will be released in two days.

Patch

diff --git a/mercurial/parsers.c b/mercurial/parsers.c
--- a/mercurial/parsers.c
+++ b/mercurial/parsers.c
@@ -1113,7 +1113,14 @@  static PyObject *compute_phases(indexObj
 	if (minrevallphases != -1) {
 		for (i = minrevallphases; i < self->raw_length; i++) {
 			data = index_deref(self, i);
-			set_phase_from_parents(phases, getbe32(data+24), getbe32(data+28), i);
+			parent_1 = getbe32(data + 24);
+			parent_2 = getbe32(data + 28);
+			if (parent_1 >= len || parent_2 >= len) {
+				PyErr_SetString(PyExc_ValueError,
+						"parent out of range");
+				goto release_phases;
+			}
+			set_phase_from_parents(phases, parent_1, parent_2, i);
 		}
 		for (i = 0; i < addlen; i++) {
 			rev = PyList_GET_ITEM(self->added, i);
@@ -1205,7 +1212,11 @@  static PyObject *index_headrevs(indexObj
 		data = index_deref(self, i);
 		parent_1 = getbe32(data + 24);
 		parent_2 = getbe32(data + 28);
-
+		if (parent_1 >= len || parent_2 >= len) {
+			PyErr_SetString(PyExc_ValueError,
+					"parent out of range");
+			goto bail;
+		}
 		if (parent_1 >= 0)
 			nothead[parent_1] = 1;
 		if (parent_2 >= 0)
diff --git a/tests/test-log.t b/tests/test-log.t
--- a/tests/test-log.t
+++ b/tests/test-log.t
@@ -2050,3 +2050,43 @@  Log -f on B should reports current chang
      summary:     A1B1C1
   
   $ cd ..
+
+Test buffer overflow by invalid parent at parsers.c. they were SEGV before.
+"hg log" for compute_phases(), "hg log -Tdefault" for index_headrevs():
+
+  $ hg init invalidparent
+  $ cd invalidparent
+  $ echo 0 >> a
+  $ hg commit -Am 0
+  adding a
+  $ echo 1 >> a
+  $ hg commit -m 1
+  $ cp .hg/store/00changelog.i .hg/store/00changelog.i.orig
+
+  $ python <<EOF
+  > data = open(".hg/store/00changelog.i.orig", "rb").read()
+  > data = data[:24] + '\x10\0\0\0' + data[28:]  # p1
+  > open(".hg/store/00changelog.i", "wb").write(data)
+  > EOF
+  $ rm -Rf .hg/cache
+  $ hg log 2>&1 | grep -v '^[ *][ *]'
+  Traceback (most recent call last):
+  ValueError: parent out of range
+  $ hg log -Tdefault 2>&1 | grep -v '^[ *][ *]'
+  Traceback (most recent call last):
+  ValueError: parent out of range
+
+  $ python <<EOF
+  > data = open(".hg/store/00changelog.i.orig", "rb").read()
+  > data = data[:28] + '\x10\0\0\0' + data[32:]  # p2
+  > open(".hg/store/00changelog.i", "wb").write(data)
+  > EOF
+  $ rm -Rf .hg/cache
+  $ hg log 2>&1 | grep -v '^[ *][ *]'
+  Traceback (most recent call last):
+  ValueError: parent out of range
+  $ hg log -Tdefault 2>&1 | grep -v '^[ *][ *]'
+  Traceback (most recent call last):
+  ValueError: parent out of range
+
+  $ cd ..