classification
Title: Unsafe memory access in PyStructSequence_InitType
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: Pasha Stetsenko, serhiy.storchaka, xiang.zhang
Priority: normal Keywords:

Created on 2018-06-03 06:37 by Pasha Stetsenko, last changed 2018-06-21 01:29 by xiang.zhang. This issue is now closed.

Messages (4)
msg318523 - (view) Author: Pasha Stetsenko (Pasha Stetsenko) Date: 2018-06-03 06:37
The documentation (https://docs.python.org/3/c-api/tuple.html) for `PyStructSequence_InitType` describes the function as follows:

> void PyStructSequence_InitType(PyTypeObject *type, PyStructSequence_Desc *desc)
> Initializes a struct sequence type `type` from `desc` in place.

And most of the time it does just that.
However, when running under python compiled in debug mode, the body of the function will contain the following code at the very beginning:
```
    if (type->ob_base.ob_base._ob_next) {
        _Py_ForgetReference((PyObject*)type);
    }
``` 
Since `type` here is a preallocated but an uninitialized piece of memory, it may contain garbage data that when interpreted as a "live" PyObject will result in memory corruption or process crash.

Thus, either the description for the `PyStructSequence_InitType` method has to document that the `type` object must be zeroed-out before being passed to the method, or the call to `_Py_ForgetReference` be removed.
msg318525 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-06-03 07:57
Could you please provide a C code that reproduces the crash?
msg318671 - (view) Author: Pasha Stetsenko (Pasha Stetsenko) Date: 2018-06-04 16:35
The code is simple:
```
// first initialize PyStructSequence_Field* fields; then:
PyTypeObject* type = malloc(sizeof(PyTypeObject));
PyStructSequence_InitType(type, desc);
```

Of course, `malloc` can accidentally allocate memory that is already filled with 0s (especially if it is run at the start of the program). So in order to make the code exhibit the bug reliably, you can add
```
memset(type, 0xDA, sizeof(PyTypeObject));
```
after the `malloc`.
msg320071 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2018-06-20 11:57
I don't think here is a problem. It crashes because you preallocate the type object in a wrong way. You should not just does a malloc and then passes it to the API. In this way, you are able to crash many APIs. For example, malloc a dictobject and then pass it to PyDict_SetItem could highly possibly crash. You should use PyDict_New to allocate the dictobject. Also here, you need to use PyType_GenericAlloc(&PyType_Type, 0) to allocate the type object, not just a malloc.
History
Date User Action Args
2018-06-21 01:29:02xiang.zhangsetstatus: open -> closed
resolution: not a bug
stage: resolved
2018-06-20 11:57:36xiang.zhangsetnosy: + xiang.zhang
messages: + msg320071
2018-06-04 16:35:59Pasha Stetsenkosetmessages: + msg318671
2018-06-03 07:57:04serhiy.storchakasetversions: + Python 3.8, - Python 3.5
nosy: + serhiy.storchaka

messages: + msg318525

components: + Interpreter Core
2018-06-03 06:37:33Pasha Stetsenkocreate