Skip to content

gh-129573: Fix possible abort from non-string suggestions in calculate_suggestions/_Py_CalculateSuggestions #130997

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

devdanzin
Copy link
Contributor

@devdanzin devdanzin commented Mar 9, 2025

This PR adds a simple check that suggestion candidates are strings in calculate_suggestions(3.12)/_Py_CalculateSuggestions(main), avoiding an abort in debug builds from code like below:

import runpy
runpy._run_module_code("blech", {0: "", "bluch": ""}, "")

The abort only happens in 3.12, because the code in main checks for non-string candidates in _suggestions__generate_suggestions_impl. However, since the affected code is still present in main, this PR is against that branch.

In main, the code above exits the new REPL, which will be reported as a new issue with associated PR.

Found using fusil by @vstinner.

@vstinner
Copy link
Member

test_traceback is failing on the CI.

@tomasr8 tomasr8 removed the needs backport to 3.12 only security fixes label Apr 10, 2025
@python-cla-bot
Copy link

python-cla-bot bot commented Apr 18, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

@@ -148,6 +148,10 @@ _Py_CalculateSuggestions(PyObject *dir,
}
for (Py_ssize_t i = 0; i < dir_size; ++i) {
PyObject *item = PyList_GET_ITEM(dir, i);
if (!PyUnicode_Check(item)) {
PyMem_Free(buffer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should you raise an exception here?

Above, if (dir_size >= MAX_CANDIDATE_ITEMS) { return NULL; } doesn't raise an exception.

@vstinner
Copy link
Member

@devdanzin: Tests are failing on your PR, and you didn't sign the CLA. What's the status of this PR?

@devdanzin
Copy link
Contributor Author

I'm sorry, I've left this one fall behind. Would you like to take it over?

I have signed the CLA and even contributed some trivial code, I'll wait to see if it's a CLA bot issue or my CLA status changed to unverified somehow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants