llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Yao Qi (qiyao) <details> <summary>Changes</summary> A `breakpoint set -P <class>` with an invalid-UTF-8 class name crashes lldb with `EXC_BAD_ACCESS`. The class name is passed through unmodified, so the byte sequence `\xd0p` (an incomplete two-byte UTF-8 sequence) reaches the Python layer: ``` ./bin/lldb -b -o $'breakpoint set -P \xd0p -f main.cpp' ./bin/lldb ... PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ Stack dump: #<!-- -->4 PythonDictionary::GetItem(PythonObject const&) const #<!-- -->6 PythonDictionary::GetItemForKey(PythonObject const&) const #<!-- -->7 PythonObject::ResolveNameWithDictionary(StringRef, PythonDictionary const&) #<!-- -->9 ScriptedPythonInterface::CreatePluginObject<...>(...) ``` `ResolveNameWithDictionary` builds a `PythonString` from the class name and looks it up in the module dictionary. When the name is not valid UTF-8, `PyUnicode_FromStringAndSize` returns `NULL`, so the `PythonString` is left with `m_py_obj == NULL`. `GetItemForKey` forwards that object to `GetItem`, which passed the `NULL` `PyObject *` straight to `PyDict_GetItemWithError`. Debugging the crashing lldb under lldb shows the `NULL` key arriving in `GetItem`: ``` (lldb) break set -n lldb_private::python::PythonDictionary::GetItem (lldb) run --no-use-colors -b -s /tmp/bp_cmds.txt * stop reason = breakpoint 1.1 (lldb) up (lldb) frame variable key.m_py_obj (PyObject *) key.m_py_obj = nullptr ``` Continuing lands on the dereference of that `NULL` inside CPython: ``` (lldb) continue * stop reason = EXC_BAD_ACCESS (code=1, address=0x8) frame #<!-- -->0: PyDict_GetItemWithError + 40 -> ldr x8, [x1, #<!-- -->0x8] ``` `x1` holds the key argument, which is `NULL`, so reading the type field at offset `0x8` faults at address `0x8`. `GetItem` already guards against an invalid dictionary (`!IsValid()`); extend that guard to reject an invalid key as well and return `nullDeref()`. `GetItemForKey` consumes the error and returns an empty `PythonObject`, so existing callers such as `ResolveNameWithDictionary` keep working and the breakpoint command now fails cleanly instead of crashing. This was found by `lldb-commandinterpreter-fuzzer`. Adds `PythonDataObjectsTest.TestPythonDictionaryGetItemWithInvalidKey`, which feeds an invalid-UTF-8 key to `GetItem`. Without the fix the test dereferences the `NULL` key and crashes (`SIGSEGV`). --- Full diff: https://github.com/llvm/llvm-project/pull/205753.diff 2 Files Affected: - (modified) lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp (+1-1) - (modified) lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp (+24) ``````````diff diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp index 800f991680c85..fba98abf9e83b 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/PythonDataObjects.cpp @@ -697,7 +697,7 @@ PythonObject PythonDictionary::GetItemForKey(const PythonObject &key) const { Expected<PythonObject> PythonDictionary::GetItem(const PythonObject &key) const { - if (!IsValid()) + if (!IsValid() || !key.IsValid()) return nullDeref(); PyObject *o = PyDict_GetItemWithError(m_py_obj, key.get()); if (PyErr_Occurred()) diff --git a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp index 0d4b04b7a1284..24ed721049e67 100644 --- a/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp +++ b/lldb/unittests/ScriptInterpreter/Python/PythonDataObjectsTests.cpp @@ -490,6 +490,30 @@ TEST_F(PythonDataObjectsTest, TestPythonDictionaryValueEquality) { EXPECT_EQ(value_1, chk_str.GetString()); } +TEST_F(PythonDataObjectsTest, TestPythonDictionaryGetItemWithInvalidKey) { + // A PythonString constructed from invalid UTF-8 silently fails (the + // PyUnicode_FromStringAndSize call returns NULL) and leaves m_py_obj == NULL. + // PythonDictionary::GetItem(const PythonObject&) must not pass that NULL key + // through to PyDict_GetItemWithError, which would then dereference it and + // crash. + llvm::StringRef invalid_utf8("\xd0p", 2); + PythonString invalid_key(invalid_utf8); + EXPECT_FALSE(invalid_key.IsValid()); + + PythonDictionary dict(PyInitialValue::Empty); + dict.SetItemForKey(PythonString("real_key"), PythonInteger(42)); + + // GetItem must return an error (not crash) when given an invalid key. + llvm::Expected<PythonObject> item = dict.GetItem(invalid_key); + EXPECT_FALSE(static_cast<bool>(item)); + llvm::consumeError(item.takeError()); + + // GetItemForKey wraps GetItem, consumes any error, and returns an empty + // PythonObject. It must not crash either. + PythonObject result = dict.GetItemForKey(invalid_key); + EXPECT_FALSE(result.IsAllocated()); +} + TEST_F(PythonDataObjectsTest, TestPythonDictionaryManipulation) { // Test that manipulation of a dictionary behaves correctly when wrapped // by a PythonDictionary. `````````` </details> https://github.com/llvm/llvm-project/pull/205753 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
