[3.12] gh-125318: Perform sanity check in find_ttinfo to mitigate out-of-bounds read
#125824
Conversation
y5c4l3
force-pushed
the
find-ttinfo-sanity-312m
branch
3 times, most recently
from
64d1fc8 to
9f62eac
Compare
User custom temporal data types that partially fulfill the datetime contract, e.g. `hour`, `minute`, `second` and `toordinal()`, could bypass the type checking in `find_ttinfo`. Accessing internal fields `PyDateTime_GET_*` on non-datetime types causes undefined behaviors and out-of-bounds read. Signed-off-by: y5c4l3 <[email protected]>
y5c4l3
force-pushed
the
find-ttinfo-sanity-312m
branch
from
9f62eac to
5d32053
Compare
| } | ||
|
|
||
| unsigned char fold = PyDateTime_DATE_GET_FOLD(dt); | ||
| assert(fold < 2); |
There was a problem hiding this comment.
Assertion here is unwanted, even though it's optimized out for some mysterious reason.
There was a problem hiding this comment.
Assertions are always optimized out :)
There was a problem hiding this comment.
If ASAN is failing, then I don't think this is the right fix. Similarly, Valgrind is angry too:
==14426== Invalid read of size 1
==14426== at 0x4877781: find_ttinfo (_zoneinfo.c:2207)
==14426== by 0x4877DC9: zoneinfo_ZoneInfo_utcoffset_impl (_zoneinfo.c:555)
==14426== by 0x4878150: zoneinfo_ZoneInfo_utcoffset (_zoneinfo.c.h:229)
==14426== by 0x2B0619: method_vectorcall_FASTCALL_KEYWORDS_METHOD (descrobject.c:387)
==14426== by 0x2A3C38: _PyObject_VectorcallTstate (pycore_call.h:92)
==14426== by 0x2A3D0C: PyObject_Vectorcall (call.c:325)
==14426== by 0x3B301E: _PyEval_EvalFrameDefault (bytecodes.c:2715)
==14426== by 0x3B9035: _PyEval_EvalFrame (pycore_ceval.h:89)
==14426== by 0x3B9153: _PyEval_Vector (ceval.c:1683)
==14426== by 0x3B9212: PyEval_EvalCode (ceval.c:578)
==14426== by 0x419165: run_eval_code_obj (pythonrun.c:1722)
==14426== by 0x41922E: run_mod (pythonrun.c:1743)
==14426== Address 0x12c374d3 is 19 bytes after a block of size 48 alloc'd
==14426== at 0x48447A8: malloc (vg_replace_malloc.c:446)
==14426== by 0x2F8836: _PyMem_RawMalloc (obmalloc.c:45)
==14426== by 0x2FA704: PyObject_Malloc (obmalloc.c:801)
==14426== by 0x3122B7: _PyType_AllocNoTrack (typeobject.c:1704)
==14426== by 0x31233C: PyType_GenericAlloc (typeobject.c:1728)
==14426== by 0x30EAE1: object_new (typeobject.c:5506)
==14426== by 0x3136BD: type_call (typeobject.c:1667)
==14426== by 0x2A3A7E: _PyObject_MakeTpCall (call.c:240)
==14426== by 0x2A3CBE: _PyObject_VectorcallTstate (pycore_call.h:90)
==14426== by 0x2A3D0C: PyObject_Vectorcall (call.c:325)
==14426== by 0x3B301E: _PyEval_EvalFrameDefault (bytecodes.c:2715)
==14426== by 0x3B9035: _PyEval_EvalFrame (pycore_ceval.h:89)
This means that it can still segfault in the wild.
Also: this shouldn't be a backport PR. This fix should go into main, and then we can backport it respectively, and then we can decide on a better fix for 3.14+ (and possibly 3.13).
|
|
||
| // gh-125318: out-of-bounds sanity check on non-PyDateTime types | ||
| if (fold >= 2) { | ||
| PyErr_Format(PyExc_MemoryError, |
There was a problem hiding this comment.
Don't use a MemoryError:
| PyErr_Format(PyExc_MemoryError, | |
| PyErr_Format(PyExc_ValueError, |
| "find_ttinfo: sanity check failed, fold = %d, expected " | ||
| "only 0 or 1", fold); |
There was a problem hiding this comment.
This isn't particularly useful to the user. Could you give more information on what they should do? (This is almost as bad as an assertion failure--we're almost certain to get reports about this.)
|
3.12 is entering security-only fixes mode today. Since this is a fix for a crash, it can be considered as a security fix in the future. In any case, it should start as a pull request against |
3 participants
User custom temporal data types that partially fulfill the datetime contract, e.g.
hour,minute,secondandtoordinal(), could bypass the type checking infind_ttinfo. Accessing internal fieldsPyDateTime_GET_*on non-datetime types causes undefined behaviors and out-of-bounds read.zoneinfowith custom DateTime class #125318