Possible segfault if globals dict is NULL in run_eval_code_obj

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

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=NULL to PyRun_FileExFlags or 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 globals is 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.

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.

gh-116180: Check the globals argument in PyRun_* C API (GH-116637)

pythongh-116180: Check the globals argument in PyRun_* C API (pythonG…

Navigation Menu

Repository navigation

Possible segfault if globals dict is NULL in run_eval_code_obj #116180

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Possible segfault if globals dict is NULL in run_eval_code_obj #116180

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

erlend-aasland commented on Mar 12, 2024

zooba commented on Apr 2, 2024

serhiy-storchaka commented on Apr 3, 2024

zooba commented on Apr 3, 2024

serhiy-storchaka commented on Apr 17, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions