Check Python C-API returns in terrain_enum builders; closes #322
WangSet.terrain_enum (and the parallel AutoRuleSet.terrain_enum) built a Python IntEnum from parsed terrain/color names but ignored the return codes of PyLong_FromLong, PyUnicode_FromString, and PyDict_SetItemString. A name carrying invalid UTF-8 -- trivially reachable from an untrusted .tsx/.ldtk import -- makes those calls fail and leave an exception pending. The code then called PyObject_Call with that error already set, tripping CPython's _PyErr_Occurred assertion and aborting (libFuzzer: deadly signal). Both functions now check every C-API return, clean up their references, and return NULL so the real exception (e.g. UnicodeDecodeError) propagates. Verified by A/B replay of the #312 fuzz_import_parsers crash corpus under clang-18 ASan/UBSan: the saved crash input aborts pre-fix inside PyWangSet::terrain_enum -> PyObject_Call and runs clean post-fix. New regression test crafts a .tsx with invalid-UTF-8 wangset/wangcolor names. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TvvE6TB2cScBVbeap2btHw
This commit is contained in:
parent
79df32317b
commit
a2c76c3643
3 changed files with 138 additions and 4 deletions
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
88
tests/regression/issue_322_terrain_enum_bad_utf8_test.py
Normal file
88
tests/regression/issue_322_terrain_enum_bad_utf8_test.py
Normal file
|
|
@ -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'<?xml version="1.0" encoding="UTF-8"?>\n'
|
||||
b'<tileset version="1.10" tiledversion="1.10.2" name="t" '
|
||||
b'tilewidth="16" tileheight="16" tilecount="4" columns="2">\n'
|
||||
b' <image source="x.png" width="32" height="32"/>\n'
|
||||
b' <wangsets>\n'
|
||||
b' <wangset name="ws_\xff\xfe" type="corner" tile="-1">\n'
|
||||
b' <wangcolor name="C_\xff\xfe" color="#ff0000" tile="-1" probability="1"/>\n'
|
||||
b' </wangset>\n'
|
||||
b' </wangsets>\n'
|
||||
b'</tileset>\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:
|
||||
# "<built-in> 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)
|
||||
Loading…
Add table
Add a link
Reference in a new issue