Conversation
|
Documentation is incoming |
ucodery
force-pushed
the
cve-2020-10735
branch
from
8400bfe to
1369f8a
Compare
| if (env && *env != '\0') { | ||
| errno = 0; | ||
| maxdigits = strtol(env, (char **)&endptr, 10); | ||
| if (*endptr != '\0' || errno == ERANGE || maxdigits < INT_MIN || maxdigits > INT_MAX || |
There was a problem hiding this comment.
should these be LONG_MIN and LONG_MAX?
There was a problem hiding this comment.
No, because maxdigits is type int. Would love it if C had strtoi, but well best practice seems to be using the standard strtol and then cast when in an acceptable range.
(edit) maxdigits is long but it is just holding the value until it gets put in Py_LongMaxStrDigits, which is type int.
| int max_str_digits = interp->long_max_str_digits; | ||
| if ((max_str_digits > 0) && (digitlen > max_str_digits)) { |
There was a problem hiding this comment.
The first conditional in line 1415 makes me wish that the max_str_digits value were unsigned. Is that at all possible?
There was a problem hiding this comment.
As you saw elsewhere, this is not really possible, as 0 is a special case for "disable this check entirely", and so -1 is used as a special case for "this was unset, use the default".
| #undef _STRINGIFY | ||
| #undef STRINGIFY | ||
| } | ||
| Py_LongMaxStrDigits = (int)maxdigits; |
There was a problem hiding this comment.
This is the other place I'd check that maxdigits is 0 or positive. Given that longobject needs -1 as a flag value, I want to be very certain to about this condition.
There was a problem hiding this comment.
I don't understand, maxdigits is checked for 0 or positive on line 4459. I suppose it is theoretically possible that _PY_LONG_MAX_STR_DIGITS_THRESHOLD is changed to a negative, but that would probably lead to multiple issues.
Note that Py_FatalError doesn't return, it will kill the process.
There was a problem hiding this comment.
I may have been misreading this conditional. To confirm then, maxdigits has be either 0 or a value greater than _PY_LONG_MAX_STR_DIGITS_THRESHOLD, or the process will abort?
That should work then.
There was a problem hiding this comment.
That's correct, the check being performed here is enforcing the interface described (at length) in stdtypes.rst
| #ifdef WITH_TSC | ||
| interp->tscdump = 0; | ||
| #endif | ||
| interp->long_max_str_digits = -1; |
3 participants
No description provided.