[CSA Bug Report]Clang Static Analyzer(CSA) Memory Bug Detected
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 35/100
Research direction
Start with rosidl_generator_py/resource/_msg_support.c.em, especially lines 179-193 and the error-return paths from lines 538 onward. Trace the ownership of name_attr, module_attr, class_attr, and _pymessage through each failure path. Done means borrowed string data is not used after its owner is released and allocated _pymessage objects are released before every NULL or false return.
Written by the indexing model from the issue text.
Description
Bug report
Clang Static Analyzer(CSA), pyrefcon @Snape3058 (http://lcs.ios.ac.cn/~maxt/PyRefcon/ASE-2023.pdf)
- Operating System:
- Linux d18de72e1bb7 5.4.0-196-generic x86
Bug Type: Access released Memory/Use After Free
File: _msg_support.c.em
Commit: https://github.com/ros2/rosidl_python/blob/1fbd99b5fe1fa04674c73e7e5992ccc88e28157c/rosidl_generator_py/resource/_msg_support.c.em#L179-L193
Detail: after Py_DECREF module_attr and class_attr may be released, module_name and class_name are possible freed, Causing Access released Memory/Use After Free.
Prove of Content(POC):
static *
poc(PyObject * object) {
PyObject * module_attr = PyObject_GetAttrString(object, "__class__");
char * module_name = NULL;
if (module_attr) {
PyObject *name_attr = PyObject_GetAttrString(module_attr, "__name__");
if (name_attr) {
module_name = (char *)PyUnicode_1BYTE_DATA(name_attr);
Py_DECREF(name_attr);
Py_DECREF(module_attr);
printf("%s", module_name);
}
}
return PyLong_FromLong(0);
}
And this is the correct result.
However, If the garbege collect was triggered between Py_DECREF(object) and usage of string module_name, things will become troublesome.
static *
poc(PyObject * object) {
PyObject * module_attr = PyObject_GetAttrString(object, "__class__");
char * module_name = NULL;
if (module_attr) {
PyObject *name_attr = PyObject_GetAttrString(module_attr, "__name__");
if (name_attr) {
module_name = (char *)PyUnicode_1BYTE_DATA(name_attr);
Py_DECREF(name_attr);
Py_DECREF(module_attr);
call_gc_collect();
printf("%s\n", module_name);
}
}
return PyLong_FromLong(0);
}
void call_gc_collect() {
PyObject *gc_module = PyImport_ImportModule("gc");
if (gc_module) {
PyObject *gc_collect = PyObject_GetAttrString(gc_module, "collect");
if (gc_collect && PyCallable_Check(gc_collect)) {
PyObject *result = PyObject_CallObject(gc_collect, NULL);
Py_XDECREF(result);
}
Py_XDECREF(gc_collect);
Py_DECREF(gc_module);
}
}
This is the result:
Finding that module_name has been freed. In this case, I manually call gc.collect() to explain it. In real python environment, GC could free module_name at any time, Causing Use After Free Bug.
How to Fix: I think it's better to use these string before Py_DECREF:
{
PyObject * class_attr = PyObject_GetAttrString(_pymsg, "__class__");
if (class_attr) {
PyObject * name_attr = PyObject_GetAttrString(class_attr, "__name__");
if (name_attr) {
class_name = (char *)PyUnicode_1BYTE_DATA(name_attr);
}
PyObject * module_attr = PyObject_GetAttrString(class_attr, "__module__");
if (module_attr) {
module_name = (char *)PyUnicode_1BYTE_DATA(module_attr);
}
if (!class_name || !module_name) {
return false;
}
snprintf(full_classname_dest, sizeof(full_classname_dest), "%s.%s", module_name, class_name);
Py_XDECREF(module_attr);
Py_XDECREF(name_attr);
Py_DECREF(class_attr);
}
}
Bug Type: Non-Zero Dead Object/Memory Leak
File: _msg_support.c.em
Commit: https://github.com/ros2/rosidl_python/blob/1fbd99b5fe1fa04674c73e7e5992ccc88e28157c/rosidl_generator_py/resource/_msg_support.c.em#L538
Detail: If _pymessage has been correctly allocated, function may return NULL without freeing _pymessage, Causing Non-Zero Dead Object/Memory Leak.
field = PyObject_GetAttrString(_pymessage, "@(member.name)");
if (!field) {
return NULL;
}
Detail: if PyObject_GetAttrString fail and return NULL, function will return NULL causing _pymessage leak.
Same in any code block fail and return NULL or false: https://github.com/ros2/rosidl_python/blob/1fbd99b5fe1fa04674c73e7e5992ccc88e28157c/rosidl_generator_py/resource/_msg_support.c.em#L576-L579
Fix: I think it's better to add Py_DECREF(_pymessage) before return NULL;
- Dominant language
- EmberScript
- Stars
- 26
- Forks
- 69
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 1
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from ros2/rosidl_python
-
enhancement
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
ros2/rosidl_python#255 · 1 reaction ·
-
bug
Difficulty 3/5 1-2 days Newbie friendliness 56/100
ros2/rosidl_python#264 · 2 comments ·
-
bug
Difficulty 4/5 3-5 days Newbie friendliness 55/100
ros2/rosidl_python#257 · 8 comments ·
-
enhancement
Difficulty 3/5 1-2 days Newbie friendliness 35/100
ros2/rosidl_python#242 ·
-
Difficulty 3/5 1-2 days Newbie friendliness 32/100
ros2/rosidl_python#219 · 2 comments ·
All issues in ros2/rosidl_python
Similar issues
-
upstream update
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
conan-io/conan-center-index#31098 ·
Maintainers usually reply within 2 days
-
cvss-severity:high devguard l3montree-cybersecurity/.../devguard-documentation pkg:devguard/l3montree-c.../devguard-documentation risk:low state:open
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
l3montree-dev/devguard-documentation#338 · 1 comment ·
-
bug runtime spec compliance
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
frostney/GocciaScript#1413 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
EverMind-AI/Raven#845 ·
Maintainers usually reply within 1 day
-
bug needs-acceptance wg/developer-experience-ecosystem
Difficulty 2/5 1-3 hours Newbie friendliness 88/100
vllm-project/semantic-router#4480 ·
Maintainers usually reply within 1 day