gh-125318: Prevent segfaults when zoneinfo is used with "false friends"#139132
gh-125318: Prevent segfaults when zoneinfo is used with "false friends"#139132pganssle wants to merge 4 commits intopython:mainfrom
Conversation
|
Not sure who might want to review this, maybe @StanFromIreland or @serhiy-storchaka or @ZeroIntensity? |
| return NULL; | ||
| } | ||
|
|
||
| fold = (unsigned char)PyLong_AsLong(fold_obj); |
There was a problem hiding this comment.
Strictly speaking, this is an undefined behavior. You can use (unsigned char)(unsigned long) if you are fine with silent ignoring the higher bits.
But assert(fold < 2) below can fail, because fold can be in range 0-255 here. We need more strict runtime check to ensure that it is only 0 or 1.
long fold_long = PyLong_AsLong(fold_obj);
Py_DECREF(fold_obj);
if (...) {...}
fold = `(unsigned char)fold_long;| return NULL; | ||
| } | ||
|
|
||
| year = PyLong_AsLong(year_obj); |
There was a problem hiding this comment.
Possible integer overflow here. Use PyLong_AsInt().
And do we need an additional range check before passing it to find_tzrule_ttinfo() or it works with the full range of int?
| int year; | ||
| if (PyDateTime_Check(dt)) { | ||
| year = PyDateTime_GET_YEAR(dt); | ||
| } else { |
There was a problem hiding this comment.
Nitpick.
| } else { | |
| } | |
| else { |
| } | ||
| } | ||
| return find_tzrule_ttinfo(&(self->tzrule_after), ts, fold, year); | ||
| } else { |
There was a problem hiding this comment.
| } else { | |
| } | |
| else { |
| unsigned char fold; | ||
| if (PyDateTime_Check(dt)) { | ||
| fold = PyDateTime_DATE_GET_FOLD(dt); | ||
| } else { |
There was a problem hiding this comment.
| } else { | |
| } | |
| else { |
| } | ||
| if (fold_int < 0 || fold_int > 2) { | ||
| PyErr_Format(PyExc_ValueError, | ||
| "fold must be 0 or 1, got %d", fold_int); |
There was a problem hiding this comment.
| "fold must be 0 or 1, got %d", fold_int); | |
| "fold must be 0 or 1, got %ld", fold_int); |
There was a problem hiding this comment.
Or use int and PyLong_AsLong() for fold_int.
|
|
||
| if (year < MINYEAR || year > 9999) { | ||
| PyErr_Format(PyExc_ValueError, | ||
| "year out of range, should be in (%d, %d) but got %d", |
There was a problem hiding this comment.
See error messages in check_date_args(), datetime_date_fromisocalendar_impl(), utc_to_seconds().
| } | ||
| if (fold_int < 0 || fold_int > 2) { | ||
| PyErr_Format(PyExc_ValueError, | ||
| "fold must be 0 or 1, got %d", fold_int); |
There was a problem hiding this comment.
Or use int and PyLong_AsLong() for fold_int.
|
This PR is stale because it has been open for 30 days with no activity. |
3 participants
Fixes #125318.
I think this can be backported to bugfix branches, since it just prevents segfaults. It might make things slower in some situations for people who have "false friend" classes that happen to have the same memory layout as
datetimebut that is not really a supported mode of operation anyway, we're really just fixing this because you shouldn't be able to crash the interpreter from pure Python code.zoneinfowith custom DateTime class #125318