[BUG] Coverity reports out-of-bound-access in C++ code generated by Cython

[oleksandr-pavlyk]

Describe the bug

Using Cython 3.0.2 and the following source code (named "my_pyx.pyx"):

# distutils: language = c++
# cython: language_level=3
# cython: linetrace=True

cdef class Foo:
    cdef double bar_

    def __cinit__(self, double bar):
        self.bar_ = bar

    cdef double get_bar(self):
        return self.bar_

    def get_square(self):
        return self.bar_ * self.bar_

    @property
    def bar(self):
        return self.get_square()

Generate C++ source file using cython -t -w $(pwd) --cplus my_pyx.pyx. The output is too large to be included verbatim in the ticket.

I built this extension: g++ my_pyx.cpp -fPIC -shared $(python3-config --includes) $(python3-config --ldflags) $(python3-config --libs) -o my_pyx.so.

Coverity scan on such extension flags a "Very high" out-of-bound access issue:

Code to reproduce the behaviour:

# distutils: language = c++
# cython: language_level=3
# cython: linetrace=True

cdef class Foo:
    cdef double bar_

    def __cinit__(self, double bar):
        self.bar_ = bar

    cdef double get_bar(self):
        return self.bar_

    def get_square(self):
        return self.bar_ * self.bar_

    @property
    def bar(self):
        return self.get_square()

Expected behaviour

No response

OS

Linux

Python version

No response

Cython version

3.0.2

Additional context

Although Coverity assigns this issue a very high priority, keep in mind it may be a false positive. Even if so, it would be nice to resolve it, since it generates many different hits if many Cython source files are used in the project.

[da-woods]

I think that's a false positive - the third argument is 0 so it's passing 0 args. However, it'd probably cost us little to make __pyx_callargs one element longer to avoid this. PR welcome

[oleksandr-pavlyk]

@da-woods A local change

diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 00cdb074c..c77439206 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -6601,11 +6601,13 @@ class PyMethodCallNode(SimpleCallNode):
         code.globalstate.use_utility_code(
             UtilityCode.load_cached("PyObjectFastCall", "ObjectHandling.c"))

+        padding_null_arg = code.funcstate.allocate_temp(py_object_type, manage_ref=True)
+        code.putln("%s = NULL;" % padding_null_arg)
         code.putln("{")
         code.putln("PyObject *__pyx_callargs[%d] = {%s, %s};" % (
-            len(args)+1,
+            len(args)+2,
             self_arg,
-            ', '.join(arg.py_result() for arg in args)))
+            ', '.join(tuple(arg.py_result() for arg in args) + (padding_null_arg,))))
         code.putln("%s = __Pyx_PyObject_FastCall(%s, __pyx_callargs+1-%s, %d+%s);" % (
             self.result(),
             function,
@@ -6614,7 +6616,9 @@ class PyMethodCallNode(SimpleCallNode):
             arg_offset_cname))

         code.put_xdecref_clear(self_arg, py_object_type)
+        code.put_xdecref_clear(padding_null_arg, py_object_type)
         code.funcstate.release_temp(self_arg)
+        code.funcstate.release_temp(padding_null_arg)
         code.funcstate.release_temp(arg_offset_cname)
         for arg in args:
             arg.generate_disposal_code(code)

does resolve the issue. If it seems agreeable, I'll open a PR.

[oleksandr-pavlyk]

I opened up a draft PR gh-5669 to discuss this change. I am open to suggestions on how to make this fix better.