Merge pull request #284 from gpshead/fix_is_namedtuple_dict_fu

Fix the C extension module to harden is_namedtuple.

3 files changed, 46 insertions, 0 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index a397ece..67514d4 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,3 +1,11 @@
+Version 3.17.5 released 2021-08-XX
+
+* Fix the C extension module to harden is_namedtuple against looks-a-likes such
+  as Mocks. Also prevent dict encoding from causing an unraised SystemError when
+  encountering a non-Dict. Noticed by running user tests against a CPython
+  interpreter with C asserts enabled (COPTS += -UNDEBUG).
+  https://github.com/simplejson/simplejson/pull/284
+
 Version 3.17.4 released 2021-08-19
 
 * Upgrade cibuildwheel
diff --git a/simplejson/_speedups.c b/simplejson/_speedups.c
index e710128..6fefa99 100644
--- a/simplejson/_speedups.c
+++ b/simplejson/_speedups.c
@@ -386,6 +386,8 @@ static int
 _is_namedtuple(PyObject *obj)
 {
     int rval = 0;
+    /* We intentionally accept anything with a duck typed _asdict method rather
+     * than requiring it to pass PyTuple_Check(obj). */
     PyObject *_asdict = PyObject_GetAttrString(obj, "_asdict");
     if (_asdict == NULL) {
         PyErr_Clear();
@@ -2853,6 +2855,15 @@ encoder_listencode_obj(PyEncoderObject *s, JSON_Accu *rval, PyObject *obj, Py_ss
                 return rv;
             newobj = PyObject_CallMethod(obj, "_asdict", NULL);
             if (newobj != NULL) {
+                if (!PyDict_Check(newobj)) {
+                    PyErr_Format(
+                        PyExc_TypeError,
+                        "_asdict() must return a dict, not %.80s",
+                        Py_TYPE(newobj)->tp_name
+                    );
+                    Py_DECREF(newobj);
+                    return -1;
+                }
                 rv = encoder_listencode_dict(s, rval, newobj, indent_level);
                 Py_DECREF(newobj);
             }
diff --git a/simplejson/tests/test_namedtuple.py b/simplejson/tests/test_namedtuple.py
index 4387894..8035a5f 100644
--- a/simplejson/tests/test_namedtuple.py
+++ b/simplejson/tests/test_namedtuple.py
@@ -4,6 +4,11 @@ import simplejson as json
 from simplejson.compat import StringIO
 
 try:
+    from unittest import mock
+except ImportError:
+    mock = None
+
+try:
     from collections import namedtuple
 except ImportError:
     class Value(tuple):
@@ -120,3 +125,25 @@ class TestNamedTuple(unittest.TestCase):
             self.assertEqual(
                 json.dumps(f({})),
                 json.dumps(f(DeadDict()), namedtuple_as_object=True))
+
+    def test_asdict_does_not_return_dict(self):
+        if not mock:
+            if hasattr(unittest, "SkipTest"):
+                raise unittest.SkipTest("unittest.mock required")
+            else:
+                print("unittest.mock not available")
+                return
+        fake = mock.Mock()
+        self.assertTrue(hasattr(fake, '_asdict'))
+        self.assertTrue(callable(fake._asdict))
+        self.assertFalse(isinstance(fake._asdict(), dict))
+        # https://github.com/simplejson/simplejson/pull/284
+        # when running under a debug build of CPython (COPTS=-UNDEBUG)
+        # a C assertion could fire due to an unchecked error of an PyDict
+        # API call on a non-dict internally in _speedups.c.  Without a debug
+        # build of CPython this test likely passes either way despite the
+        # potential for internal data corruption.  Getting it to crash in
+        # a debug build is not always easy either as it requires an
+        # assert(!PyErr_Occurred()) that could fire later on.
+        with self.assertRaises(TypeError):
+            json.dumps({23: fake}, namedtuple_as_object=True, for_json=False)