Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Missing error checking for overflowing integer casts #2786

Open
hawkinsp opened this issue Jan 11, 2021 · 4 comments · May be fixed by #2802
Open

[BUG] Missing error checking for overflowing integer casts #2786

hawkinsp opened this issue Jan 11, 2021 · 4 comments · May be fixed by #2802
Assignees
Labels

Comments

@hawkinsp
Copy link
Contributor

Issue description

There appears to be some missing error checking around Python int to C++ long casts that may overflow. In the example below, since the Python -> C++ cast fails, I expect a C++ exception to be thrown. However, it appears the Python interpreter's error state ends up set but pybind11 fails to notice.

I suspect that this method needs to check the interpreter's error state (since PyLong_AsLong may fail if the integer is larger than a C long)

? (T) PyLong_AsLong(m_ptr)

Reproducible example code

#include <pybind11/pybind11.h>
#include <iostream>

int docast(pybind11::object a) {
  int b = pybind11::cast<pybind11::int_>(a);
  std::cerr << "Should not reach here" << std::endl;
  return b;
}

PYBIND11_MODULE(t, m) {
    m.def("docast", &docast, "Perform a cast");
}
In [1]: import t
In [2]: t.docast(3123412423423423234234234)
Should not reach here
---------------------------------------------------------------------------
OverflowError                             Traceback (most recent call last)
OverflowError: Python int too large to convert to C long

The above exception was the direct cause of the following exception:

SystemError                               Traceback (most recent call last)
<ipython-input-3-688cef4b7a48> in <module>
----> 1 t.docast(3123412423423423234234234)

SystemError: <built-in method docast of PyCapsule object at 0x102917720> returned a result with an error set
@YannickJadoul
Copy link
Collaborator

Huh, that's funny. What version of pybind11 are you on?

This seems very similar to #2336, but not really.

@hawkinsp
Copy link
Contributor Author

I just verified this with 2.6.1, which appears to be current on Pypi.

@YannickJadoul
Copy link
Collaborator

Urgh. Let me check, then. Thanks for reporting, already!

@YannickJadoul
Copy link
Collaborator

The error seems to be in the implicit conversion from py::int_ to int:

    template <typename T,
              detail::enable_if_t<std::is_integral<T>::value, int> = 0>
    operator T() const {
        return std::is_unsigned<T>::value
            ? detail::as_unsigned<T>(m_ptr)
            : sizeof(T) <= sizeof(long)
              ? (T) PyLong_AsLong(m_ptr)
              : (T) PYBIND11_LONG_AS_LONGLONG(m_ptr);
    }

This should definitely throw an exception if an error occurs. The fix seems reasonably trivial (check for -1 and PyErr_Occurred() and throw error_already_set, if so). However, I need to make some tests first, to make sure we fix this and don't regress. Somewhere this week should be possible to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants