Repository: python/cpython
Pull request: gh-98354: Add unicode check for 'name' attribute in _imp_create_builtin
Fixes #98354 

<!-- gh-issue-number: gh-98354 -->
* Issue: gh-98354
<!-- /gh-issue-number -->

Automerge-Triggered-By: GH:ericsnowcurrently

Problem statement:
create_builtin() in _imp module trigger segfault if taking a builtin object as input
<!--
  Use this template for hard crashes of the interpreter, segmentation faults, failed C-level assertions, and similar.
  Do not submit this form if you encounter an exception being unexpectedly raised from a Python function.
  Most of the time, these should be filed as bugs, rather than crashes.

  The CPython interpreter is itself written in a different programming language, C.
  For CPython, a "crash" is when Python itself fails, leading to a traceback in the C stack.
-->

# Crash report
In the following test program, _imp.create_builtin takes a object A as input. The object instance the name attribute as "self" at "self.name = self". This action triggers a segfault on CPython 3.10.7 and CPython 3.10.7. Similarly, if self.name = other keywords, e.g.,int, print, the program also crashes. It may need a checker for  _imp.create_builtin to avoid keywords.


```
import _imp

class FakeSpec:
	def __init__(self, name):
		self.name = self

A = FakeSpec("time")

imp_time = _imp.create_builtin(A)
```


# Error messages


Expected behavior on CPython 3.9.0
```
Traceback (most recent call last):
  File "/home/xxm/Desktop/imp.py", line 9, in <module>
    imp_time = _imp.create_builtin(A)
TypeError: bad argument type for built-in operation
```

Unexpected Behavior on CPython 3.10.8
Segmentation fault(core dumped)  

# Your environment

<!-- Include as many relevant details as possible about the environment you experienced the bug in -->

- CPython versions tested on:Python 3.10.8, Python 3.10.7

- Operating system and architecture: [GCC 7.5.0] on linux

<!--
You can freely edit this text. Remove any lines you believe are unnecessary.
-->

Hints from issue discussion:
Crashes on 3.12.0a0 with the following message:
```
Objects/unicodeobject.c:498: _PyUnicode_CheckConsistency: Assertion failed: PyType_HasFeature((Py_TYPE(((PyObject*)((op))))), ((1UL << 28)))
Enable tracemalloc to get the memory block allocation traceback

object address  : 0x7ffff77eb530
object refcount : 4
object type     : 0x555555c2b9b0
object type name: FakeSpec
object repr     : <__main__.FakeSpec object at 0x7ffff77eb530>

Fatal Python error: _PyObject_AssertFailed: _PyObject_AssertFailed
Python runtime state: initialized

Current thread 0x00007ffff7cad740 (most recent call first):
  File "/home/.../cpython/crash_98354.py", line 9 in <module>

Program received signal SIGABRT, Aborted.
```

Looks like `_imp_create_builtin` should check `name` for being a `str` instance in general, not only to avoid it being a keyword.

Commit message: Merge branch 'main' into _imp_create_builtin-check-name-for-unicode

Patch to review:
diff --git a/Python/import.c b/Python/import.c
index 698ef37ce0a131..c85868c31b5ae2 100644
--- a/Python/import.c
+++ b/Python/import.c
@@ -1021,6 +1021,13 @@ _imp_create_builtin(PyObject *module, PyObject *spec)
         return NULL;
     }
 
+    if (!PyUnicode_Check(name)) {
+        PyErr_Format(PyExc_TypeError,
+                     "name must be string, not %.200s",
+                     Py_TYPE(name)->tp_name);
+        return NULL;
+    }
+
     PyObject *mod = create_builtin(tstate, name, spec);
     Py_DECREF(name);
     return mod;


Code review comments:
diff_hunk: @@ -1021,6 +1021,13 @@ _imp_create_builtin(PyObject *module, PyObject *spec)
         return NULL;
     }
 
+    if (!PyUnicode_Check(name)) {
+        PyErr_Format(PyExc_TypeError,
+                     "name must be string, not %.200s",
+                     Py_TYPE(name)->tp_name);
+        return NULL;
path: Python/import.c
text: @user1:
I'm not an expert on importlib, but I believe this needs a Py_DECREF(name), and it probably also needs a regression test.

@author:
Done.