Patchwork [STABLE] parse_index2: fix crash on bad argument type (issue4110)

login
register
mail settings
Submitter Chris Jerdonek
Date Nov. 27, 2013, 1:05 a.m.
Message ID <1f9bfbc7d1152220c3b4.1385514335@stonewall.local>
Download mbox | patch
Permalink /patch/3177/
State Accepted
Headers show

Comments

Chris Jerdonek - Nov. 27, 2013, 1:05 a.m.
# HG changeset patch
# User Chris Jerdonek <chris.jerdonek@gmail.com>
# Date 1385511262 28800
#      Tue Nov 26 16:14:22 2013 -0800
# Node ID 1f9bfbc7d1152220c3b407eea26ea184eee513d1
# Parent  1c46b18b0e1c47fa4cecf21b78c083a54ae9903f
parse_index2: fix crash on bad argument type (issue4110)

Passing a non-string to parsers.parse_index2() causes Mercurial to crash
instead of raising a TypeError (found on Mac OS X 10.8.5, Python 2.7.6):

    import mercurial.parsers as parsers
    parsers.parse_index2(0, 0)

    Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
    0 parsers.so  0x000000010e071c59 _index_clearcaches + 73 (parsers.c:644)
    1 parsers.so  0x000000010e06f2d5 index_dealloc + 21 (parsers.c:1767)
    2 parsers.so  0x000000010e074e3b parse_index2 + 347 (parsers.c:1891)
    3 org.python.python 0x000000010dda8b17 PyEval_EvalFrameEx + 9911

This happens because when arguments of the wrong type are passed to
parsers.parse_index2(), indexType's initialization function index_init() in
parsers.c leaves the indexObject instance in a state that indexType's
destructor function index_dealloc() cannot handle.

This patch moves enough of the indexObject initialization code inside
index_init() from after the argument validation code to before it.
This way, when bad arguments are passed to index_init(), the destructor
doesn't crash and the existing code to raise a TypeError works.  This
patch also adds a test to check that a TypeError is raised.

Patch

diff --git a/mercurial/parsers.c b/mercurial/parsers.c
--- a/mercurial/parsers.c
+++ b/mercurial/parsers.c
@@ -1713,6 +1713,15 @@ 
 	PyObject *data_obj, *inlined_obj;
 	Py_ssize_t size;
 
+	/* Initialize before argument-checking to avoid index_dealloc() crash. */
+	self->raw_length = 0;
+	self->added = NULL;
+	self->cache = NULL;
+	self->data = NULL;
+	self->headrevs = NULL;
+	self->nt = NULL;
+	self->offsets = NULL;
+
 	if (!PyArg_ParseTuple(args, "OO", &data_obj, &inlined_obj))
 		return -1;
 	if (!PyString_Check(data_obj)) {
@@ -1723,12 +1732,7 @@ 
 
 	self->inlined = inlined_obj && PyObject_IsTrue(inlined_obj);
 	self->data = data_obj;
-	self->cache = NULL;
 
-	self->added = NULL;
-	self->headrevs = NULL;
-	self->offsets = NULL;
-	self->nt = NULL;
 	self->ntlength = self->ntcapacity = 0;
 	self->ntdepth = self->ntsplits = 0;
 	self->ntlookups = self->ntmisses = 0;
@@ -1764,7 +1768,7 @@ 
 static void index_dealloc(indexObject *self)
 {
 	_index_clearcaches(self);
-	Py_DECREF(self->data);
+	Py_XDECREF(self->data);
 	Py_XDECREF(self->added);
 	PyObject_Del(self);
 }
diff --git a/tests/test-parseindex2.py b/tests/test-parseindex2.py
--- a/tests/test-parseindex2.py
+++ b/tests/test-parseindex2.py
@@ -98,6 +98,14 @@ 
     return list(index), chunkcache
 
 def runtest() :
+    # Check that parse_index2() raises TypeError on bad arguments.
+    try:
+        parse_index2(0, True)
+    except TypeError:
+        pass
+    else:
+        print "Expected to get TypeError."
+
     py_res_1 = py_parseindex(data_inlined, True)
     c_res_1 = parse_index2(data_inlined, True)