Repository navigation
Possible segfault if globals dict is NULL in run_eval_code_obj #116180
Description
Activity
And no one encounted with such faults, so the patch will have cosmetic effect (is on the way).
It would be nice if you could come up with a reproducer, either in Python, the C API, or both.
This looks pretty trivially reproducible - just pass
globals=NULLtoPyRun_FileExFlagsor any of the functions that wrap it up. They all just pass through without checking.Prior to d6c33fb there was a NULL check deeper in that raised an error. The place where it was no longer exists, but we should probably add the check closer to where it's being used.
There are two ways to fix this issue:
- Make the code working with
globals = NULL. The part of the code already supports this case, you should add workarounds for other parts. - If this case should not be supported, make the code raising an exception with clear and precise error message. Why
globalsis NULL? If it is passed by the user of the C API, the arguments should be validated and SystemError should be passed. If it is the result of other operations, it should be checked after performing these operations, and the error message should be more specific.
You perhaps need to try both ways to determine what way is more correct.
Reacted by Erlend E. Aasland- Make the code working with
The fix is to bring back the null check that Mark deleted in his refactoring. It used to be in
_PyEval_EvalCode:if (globals == NULL) { _PyErr_SetString(tstate, PyExc_SystemError, "PyEval_EvalCodeEx: NULL globals"); return NULL; }(See lines 4386-4390 of ceval.c in d6c33fb - sorry Github won't let me link directly for some reason)
After working on the proposed PR and tests, I think that it is not a bug. The lack of the validation in the C API is not a bug. You are supposed to pass valid arguments, and if it fails for invalid arguments, it is your fault.
Of course, it would be nice to handle the user error and raise a SystemError, but this is a new feature, not a bug fix. This API is high-level, so adding few new checks will not add much overhead.
Bug report
Bug description:
Possible interpreter crash condition was found by Linux Verification Center (portal.linuxtesting.ru) with SVACE.
run_eval_code_obj is called with globals dict passed as pointer without NULL check and that may cause segmentation fault in
PyEval_EvalCode->_PyEval_BuiltinsFromGlobals->PyDict_GetItemWithError line 2272 in PyDict_Check(op).
Some functions, for example, _run_script or builtin_eval_impl have globals NULL check.
Other, like PyRun_SimpleStringFlags,
PyRun_InteractiveOneObjectEx or PyRun_FileExFlags just use the pointer from the PyObject struct.
For my understanding the globals dict should always be present and it is an error condition, when it is NULL.
And no one encounted with such faults, so the patch will have cosmetic effect (is on the way).
CPython versions tested on:
3.10
Operating systems tested on:
Linux
Linked PRs