diff --git a/src/ldtk/PyAutoRuleSet.cpp b/src/ldtk/PyAutoRuleSet.cpp index 5d23163..45c7aa2 100644 --- a/src/ldtk/PyAutoRuleSet.cpp +++ b/src/ldtk/PyAutoRuleSet.cpp @@ -129,7 +129,12 @@ PyObject* PyAutoRuleSet::terrain_enum(PyAutoRuleSetObject* self, PyObject*) { // NONE = 0 (empty terrain) PyObject* zero = PyLong_FromLong(0); - PyDict_SetItemString(members, "NONE", zero); + if (!zero || PyDict_SetItemString(members, "NONE", zero) < 0) { + Py_XDECREF(zero); + Py_DECREF(members); + Py_DECREF(int_enum); + return NULL; + } Py_DECREF(zero); for (const auto& v : rs.intgrid_values) { @@ -139,15 +144,33 @@ PyObject* PyAutoRuleSet::terrain_enum(PyAutoRuleSetObject* self, PyObject*) { key = "VALUE_" + std::to_string(v.value); } PyObject* val = PyLong_FromLong(v.value); - PyDict_SetItemString(members, key.c_str(), val); + // An intgrid value name carrying invalid UTF-8 (untrusted import data) + // makes PyDict_SetItemString fail and leaves an exception pending; bail + // now rather than calling more Python API with that error set, which + // trips a _PyErr_Occurred assertion / abort. #322 + if (!val || PyDict_SetItemString(members, key.c_str(), val) < 0) { + Py_XDECREF(val); + Py_DECREF(members); + Py_DECREF(int_enum); + return NULL; + } Py_DECREF(val); } // Create enum class: IntEnum(rs.name, members) PyObject* name = PyUnicode_FromString(rs.name.c_str()); + if (!name) { + Py_DECREF(members); + Py_DECREF(int_enum); + return NULL; + } PyObject* args = PyTuple_Pack(2, name, members); Py_DECREF(name); Py_DECREF(members); + if (!args) { + Py_DECREF(int_enum); + return NULL; + } PyObject* enum_class = PyObject_Call(int_enum, args, NULL); Py_DECREF(args); diff --git a/src/tiled/PyWangSet.cpp b/src/tiled/PyWangSet.cpp index 592688c..15ec496 100644 --- a/src/tiled/PyWangSet.cpp +++ b/src/tiled/PyWangSet.cpp @@ -129,21 +129,44 @@ PyObject* PyWangSet::terrain_enum(PyWangSetObject* self, PyObject*) { // NONE = 0 (unset terrain) PyObject* zero = PyLong_FromLong(0); - PyDict_SetItemString(members, "NONE", zero); + if (!zero || PyDict_SetItemString(members, "NONE", zero) < 0) { + Py_XDECREF(zero); + Py_DECREF(members); + Py_DECREF(int_enum); + return NULL; + } Py_DECREF(zero); for (const auto& wc : ws.colors) { std::string key = toUpperSnakeCase(wc.name); PyObject* val = PyLong_FromLong(wc.index); - PyDict_SetItemString(members, key.c_str(), val); + // A wang color name carrying invalid UTF-8 (untrusted import data) makes + // PyDict_SetItemString fail and leaves an exception pending; bail now + // rather than calling more Python API with that error set, which trips a + // _PyErr_Occurred assertion / abort. #322 + if (!val || PyDict_SetItemString(members, key.c_str(), val) < 0) { + Py_XDECREF(val); + Py_DECREF(members); + Py_DECREF(int_enum); + return NULL; + } Py_DECREF(val); } // Create enum class: IntEnum(ws.name, members) PyObject* name = PyUnicode_FromString(ws.name.c_str()); + if (!name) { + Py_DECREF(members); + Py_DECREF(int_enum); + return NULL; + } PyObject* args = PyTuple_Pack(2, name, members); Py_DECREF(name); Py_DECREF(members); + if (!args) { + Py_DECREF(int_enum); + return NULL; + } PyObject* enum_class = PyObject_Call(int_enum, args, NULL); Py_DECREF(args); diff --git a/tests/regression/issue_322_terrain_enum_bad_utf8_test.py b/tests/regression/issue_322_terrain_enum_bad_utf8_test.py new file mode 100644 index 0000000..88a1fcb --- /dev/null +++ b/tests/regression/issue_322_terrain_enum_bad_utf8_test.py @@ -0,0 +1,88 @@ +""" +Regression test for issue #322 -- WangSet.terrain_enum pending-error abort. + +terrain_enum() builds a Python IntEnum from parsed Wang color names but did not +check the return of PyDict_SetItemString / PyUnicode_FromString. A name carrying +invalid UTF-8 (trivially reachable from an untrusted .tsx import) makes those +calls fail and leave an exception pending; the code then called PyObject_Call +with that error set, tripping CPython's "_PyErr_Occurred" assertion / abort. + +The fix checks every C-API return and propagates the real exception instead. +This test crafts a .tsx whose wangset and wangcolor names contain raw 0xFF/0xFE +bytes (rapidxml passes attribute bytes through verbatim), loads it, and asserts +terrain_enum() raises a clean catchable exception rather than aborting. + +ASCII-only source. Prints PASS/FAIL and sys.exit(0/1). +""" + +import mcrfpy +import sys +import os + +failures = [] + + +def check(label, cond): + if not cond: + failures.append(label) + print((" ok " if cond else " FAIL ") + label) + + +TMP = os.environ.get("TMPDIR", "/tmp") +path = os.path.join(TMP, "issue_322_badwang_%d.tsx" % os.getpid()) + +# Well-formed XML; the wangset/wangcolor name attributes carry invalid UTF-8. +xml = ( + b'\n' + b'\n' + b' \n' + b' \n' + b' \n' + b' \n' + b' \n' + b' \n' + b'\n' +) +with open(path, "wb") as fh: + fh.write(xml) + +try: + ts = mcrfpy.TileSetFile(path) + sets = list(ts.wang_sets) + check("malformed-name tileset loads and exposes its wangset", len(sets) >= 1) + + handled = 0 + for ws in sets: + try: + ws.terrain_enum() + # A clean enum is acceptable too (some platforms may decode the + # bytes); the bug was the abort, not the specific outcome. + handled += 1 + check("terrain_enum() returned without crashing", True) + except UnicodeDecodeError: + handled += 1 + check("terrain_enum() raises a clean UnicodeDecodeError (no abort)", True) + except (ValueError, TypeError) as e: + handled += 1 + check("terrain_enum() raises clean %s (no abort)" % type(e).__name__, True) + except SystemError as e: + # " returned a result with an exception set" is exactly the + # pre-fix symptom of calling Python with a pending error. + check("terrain_enum() leaked a pending error (SystemError): %s" % e, False) + check("processed at least one wangset without process abort", handled >= 1) +finally: + try: + os.unlink(path) + except OSError: + pass + + +print("") +if failures: + print("FAIL -- %d check(s) failed:" % len(failures)) + for f in failures: + print(" - " + f) + sys.exit(1) +print("PASS -- terrain_enum surfaces a clean exception for invalid-UTF-8 names.") +sys.exit(0)