From 2fbc9cea2eece673ad3288518bd1129008182742 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Sat, 3 Sep 2022 17:07:54 +0100 Subject: [PATCH 01/13] Add pre-check for int-to-str conversion --- Objects/longobject.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 6c6e2ea919682e..8b2b47dbfd91c0 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -37,6 +37,7 @@ medium_value(PyLongObject *x) #define IS_SMALL_UINT(ival) ((ival) < _PY_NSMALLPOSINTS) #define _MAX_STR_DIGITS_ERROR_FMT "Exceeds the limit (%d) for integer string conversion: value has %zd digits" +#define _MAX_STR_DIGITS_ERROR_FMT2 "Exceeds the limit (%d) for integer string conversion" static inline void _Py_DECREF_INT(PyLongObject *op) @@ -1758,6 +1759,20 @@ long_to_decimal_string_internal(PyObject *aa, size_a = Py_ABS(Py_SIZE(a)); negative = Py_SIZE(a) < 0; + /* quick and dirty pre-check for overflowing the digit limit, + based on the inequality 10/3 >= log2(10) */ + if (size_a >= 10 * _PY_LONG_MAX_STR_DIGITS_THRESHOLD + / (3 * PyLong_SHIFT) + 2) { + PyInterpreterState *interp = _PyInterpreterState_GET(); + int max_str_digits = interp->int_max_str_digits; + if ((max_str_digits > 0) && (size_a >= 10 * max_str_digits + / (3 * PyLong_SHIFT) + 2)) { + PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT2, + max_str_digits); + return -1; + } + } + /* quick and dirty upper bound for the number of digits required to express a in base _PyLong_DECIMAL_BASE: @@ -1823,8 +1838,8 @@ long_to_decimal_string_internal(PyObject *aa, Py_ssize_t strlen_nosign = strlen - negative; if ((max_str_digits > 0) && (strlen_nosign > max_str_digits)) { Py_DECREF(scratch); - PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT, - max_str_digits, strlen_nosign); + PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT2, + max_str_digits); return -1; } } From d16db91dcccc7b0e5fb8b7c0f339190132b7c042 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Sat, 3 Sep 2022 17:24:04 +0100 Subject: [PATCH 02/13] Avoid potential undefined behaviour from overflow --- Objects/longobject.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 8b2b47dbfd91c0..24487fae5edc05 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -1765,8 +1765,10 @@ long_to_decimal_string_internal(PyObject *aa, / (3 * PyLong_SHIFT) + 2) { PyInterpreterState *interp = _PyInterpreterState_GET(); int max_str_digits = interp->int_max_str_digits; - if ((max_str_digits > 0) && (size_a >= 10 * max_str_digits - / (3 * PyLong_SHIFT) + 2)) { + if ((max_str_digits > 0) && + /* avoid overflow in 10 * max_str_digits */ + (max_str_digits <= INT_MAX / 10) && + (size_a >= 10 * max_str_digits / (3 * PyLong_SHIFT) + 2)) { PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT2, max_str_digits); return -1; From 66a07ac000546ea6e5ead8498ccfeb32b6b76f13 Mon Sep 17 00:00:00 2001 From: Mark Dickinson Date: Sat, 3 Sep 2022 17:46:33 +0100 Subject: [PATCH 03/13] Reworked, even cruder bound, that avoids potential issues with integer overflow --- Objects/longobject.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 24487fae5edc05..11de375acfdea9 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -1766,9 +1766,7 @@ long_to_decimal_string_internal(PyObject *aa, PyInterpreterState *interp = _PyInterpreterState_GET(); int max_str_digits = interp->int_max_str_digits; if ((max_str_digits > 0) && - /* avoid overflow in 10 * max_str_digits */ - (max_str_digits <= INT_MAX / 10) && - (size_a >= 10 * max_str_digits / (3 * PyLong_SHIFT) + 2)) { + (max_str_digits / (3 * PyLong_SHIFT) <= (size_a - 11) / 10)) { PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT2, max_str_digits); return -1; From 6eadbdaef34212b47b729bf77d59ad83bb387611 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sat, 3 Sep 2022 21:38:14 -0700 Subject: [PATCH 04/13] Rename the error message constants. --- Objects/longobject.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index 11de375acfdea9..c825bbaa7c601b 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -36,8 +36,8 @@ medium_value(PyLongObject *x) #define IS_SMALL_INT(ival) (-_PY_NSMALLNEGINTS <= (ival) && (ival) < _PY_NSMALLPOSINTS) #define IS_SMALL_UINT(ival) ((ival) < _PY_NSMALLPOSINTS) -#define _MAX_STR_DIGITS_ERROR_FMT "Exceeds the limit (%d) for integer string conversion: value has %zd digits" -#define _MAX_STR_DIGITS_ERROR_FMT2 "Exceeds the limit (%d) for integer string conversion" +#define _MAX_STR_DIGITS_ERROR_FMT_TO_INT "Exceeds the limit (%d) for integer string conversion: value has %zd digits" +#define _MAX_STR_DIGITS_ERROR_FMT_TO_STR "Exceeds the limit (%d) for integer string conversion" static inline void _Py_DECREF_INT(PyLongObject *op) @@ -1767,7 +1767,7 @@ long_to_decimal_string_internal(PyObject *aa, int max_str_digits = interp->int_max_str_digits; if ((max_str_digits > 0) && (max_str_digits / (3 * PyLong_SHIFT) <= (size_a - 11) / 10)) { - PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT2, + PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT_TO_STR, max_str_digits); return -1; } @@ -1838,7 +1838,7 @@ long_to_decimal_string_internal(PyObject *aa, Py_ssize_t strlen_nosign = strlen - negative; if ((max_str_digits > 0) && (strlen_nosign > max_str_digits)) { Py_DECREF(scratch); - PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT2, + PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT_TO_STR, max_str_digits); return -1; } @@ -2513,7 +2513,7 @@ digit beyond the first. PyInterpreterState *interp = _PyInterpreterState_GET(); int max_str_digits = interp->int_max_str_digits; if ((max_str_digits > 0) && (digits > max_str_digits)) { - PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT, + PyErr_Format(PyExc_ValueError, _MAX_STR_DIGITS_ERROR_FMT_TO_INT, max_str_digits, digits); return NULL; } From 195686ca965a5097e4b22d4682e6938ad6baa7c1 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sat, 3 Sep 2022 21:38:30 -0700 Subject: [PATCH 05/13] Add a DoS prevention success timed regression test. --- Lib/test/test_int.py | 72 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/Lib/test/test_int.py b/Lib/test/test_int.py index e9561b02fcac7b..47fd24fed3781d 100644 --- a/Lib/test/test_int.py +++ b/Lib/test/test_int.py @@ -1,4 +1,5 @@ import sys +import time import unittest from test import support @@ -632,6 +633,77 @@ def test_max_str_digits(self): with self.assertRaises(ValueError): str(i) + def test_denial_of_service_prevented_int_to_str(self): + """Regression test: ensure we fail before performing O(N**2) work.""" + maxdigits = sys.get_int_max_str_digits() + assert maxdigits < 100_000, maxdigits # A test prerequisite. + process_time = time.process_time + + huge_int = int(f'0x{"c"*100_000}', base=16) # 120412 decimal digits. + with support.adjust_int_max_str_digits(120_412): + start = process_time() + huge_decimal = str(huge_int) + seconds_to_convert = process_time() - start + self.assertEqual(len(huge_decimal), 120_412) + # Ensuring that we chose a slow enough conversion to time. + # Unlikely any CPU core will ever be faster than the assertion. + # It takes 0.25 seconds on a Zen based cloud VM in an opt build. + self.assertGreater(seconds_to_convert, 0.02, + msg="'We're gonna need a bigger boat (int).'") + + with self.assertRaises(ValueError) as err: + start = process_time() + str(huge_int) + seconds_to_fail_huge = process_time() - start + self.assertIn('conversion', str(err.exception)) + self.assertLess(seconds_to_fail_huge, seconds_to_convert/8) + + # Now we test that a conversion that would take 30x as long also fails + # in a similarly fast fashion. + extra_huge_int = int(f'0x{"c"*500_000}', base=16) # 602060 digits. + with self.assertRaises(ValueError) as err: + start = process_time() + # If not limited, 8 seconds said Zen based cloud VM. + str(extra_huge_int) + seconds_to_fail_extra_huge = process_time() - start + self.assertIn('conversion', str(err.exception)) + self.assertLess(seconds_to_fail_extra_huge, seconds_to_convert/8) + + def test_denial_of_service_prevented_str_to_int(self): + """Regression test: ensure we fail before performing O(N**2) work.""" + maxdigits = sys.get_int_max_str_digits() + assert maxdigits < 100_000, maxdigits # A test prerequisite. + process_time = time.process_time + + huge = '8'*200_000 + with support.adjust_int_max_str_digits(200_000): + start = process_time() + int(huge) + seconds_to_convert = process_time() - start + # Ensuring that we chose a slow enough conversion to time. + # Unlikely any CPU core will ever be faster than the assertion. + # It takes 0.25 seconds on a Zen based cloud VM in an opt build. + self.assertGreater(seconds_to_convert, 0.02, + msg="'We're gonna need a bigger boat (str).'") + + with self.assertRaises(ValueError) as err: + start = process_time() + int(huge) + seconds_to_fail_huge = process_time() - start + self.assertIn('conversion', str(err.exception)) + self.assertLess(seconds_to_fail_huge, seconds_to_convert/8) + + # Now we test that a conversion that would take 30x as long also fails + # in a similarly fast fashion. + extra_huge = '7'*1_200_000 + with self.assertRaises(ValueError) as err: + start = process_time() + # If not limited, 8 seconds said Zen based cloud VM. + int(extra_huge) + seconds_to_fail_extra_huge = process_time() - start + self.assertIn('conversion', str(err.exception)) + self.assertLess(seconds_to_fail_extra_huge, seconds_to_convert/8) + def test_power_of_two_bases_unlimited(self): """The limit does not apply to power of 2 bases.""" maxdigits = sys.get_int_max_str_digits() From 87bd23d3df3aca81b88a5f245e70fded7055ab92 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sat, 3 Sep 2022 22:35:00 -0700 Subject: [PATCH 06/13] Improve the test to check close to the limit. --- Lib/test/test_int.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/Lib/test/test_int.py b/Lib/test/test_int.py index 47fd24fed3781d..2f06504d6954f2 100644 --- a/Lib/test/test_int.py +++ b/Lib/test/test_int.py @@ -651,10 +651,13 @@ def test_denial_of_service_prevented_int_to_str(self): self.assertGreater(seconds_to_convert, 0.02, msg="'We're gonna need a bigger boat (int).'") - with self.assertRaises(ValueError) as err: - start = process_time() - str(huge_int) - seconds_to_fail_huge = process_time() - start + # We test with the limit almost at the size needed to check performance. + # The performant limit check is slightly fuzzy, give it a some room. + with support.adjust_int_max_str_digits(int(.995 * 120_412)): + with self.assertRaises(ValueError) as err: + start = process_time() + str(huge_int) + seconds_to_fail_huge = process_time() - start self.assertIn('conversion', str(err.exception)) self.assertLess(seconds_to_fail_huge, seconds_to_convert/8) @@ -686,10 +689,11 @@ def test_denial_of_service_prevented_str_to_int(self): self.assertGreater(seconds_to_convert, 0.02, msg="'We're gonna need a bigger boat (str).'") - with self.assertRaises(ValueError) as err: - start = process_time() - int(huge) - seconds_to_fail_huge = process_time() - start + with support.adjust_int_max_str_digits(200_000 - 1): + with self.assertRaises(ValueError) as err: + start = process_time() + int(huge) + seconds_to_fail_huge = process_time() - start self.assertIn('conversion', str(err.exception)) self.assertLess(seconds_to_fail_huge, seconds_to_convert/8) From de9ed4d81b40f2080110a8ca13994e29caacec9f Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sat, 3 Sep 2022 22:51:22 -0700 Subject: [PATCH 07/13] Use fewer digits in the test to speed it up on slow hosts. A RPi4 takes 10 seconds for the int_to_str test now. --- Lib/test/test_int.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Lib/test/test_int.py b/Lib/test/test_int.py index 2f06504d6954f2..3b9680d656f58f 100644 --- a/Lib/test/test_int.py +++ b/Lib/test/test_int.py @@ -636,24 +636,25 @@ def test_max_str_digits(self): def test_denial_of_service_prevented_int_to_str(self): """Regression test: ensure we fail before performing O(N**2) work.""" maxdigits = sys.get_int_max_str_digits() - assert maxdigits < 100_000, maxdigits # A test prerequisite. + assert maxdigits < 50_000, maxdigits # A test prerequisite. process_time = time.process_time - huge_int = int(f'0x{"c"*100_000}', base=16) # 120412 decimal digits. - with support.adjust_int_max_str_digits(120_412): + huge_int = int(f'0x{"c"*65_000}', base=16) # 78268 decimal digits. + digits = 78_268 + with support.adjust_int_max_str_digits(digits): start = process_time() huge_decimal = str(huge_int) seconds_to_convert = process_time() - start - self.assertEqual(len(huge_decimal), 120_412) + self.assertEqual(len(huge_decimal), digits) # Ensuring that we chose a slow enough conversion to time. # Unlikely any CPU core will ever be faster than the assertion. - # It takes 0.25 seconds on a Zen based cloud VM in an opt build. - self.assertGreater(seconds_to_convert, 0.02, + # It takes 0.10 seconds on a Zen based cloud VM in an opt build. + self.assertGreater(seconds_to_convert, 0.005, msg="'We're gonna need a bigger boat (int).'") # We test with the limit almost at the size needed to check performance. # The performant limit check is slightly fuzzy, give it a some room. - with support.adjust_int_max_str_digits(int(.995 * 120_412)): + with support.adjust_int_max_str_digits(int(.995 * digits)): with self.assertRaises(ValueError) as err: start = process_time() str(huge_int) From dbd8da99c2d77dac9bf8379035fc39c9761636d5 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sat, 3 Sep 2022 23:00:58 -0700 Subject: [PATCH 08/13] Misc: Fix a typo in the header comment. --- Include/internal/pycore_long.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_long.h b/Include/internal/pycore_long.h index 76c9a5fcecfabc..30c97b7edc98e1 100644 --- a/Include/internal/pycore_long.h +++ b/Include/internal/pycore_long.h @@ -18,9 +18,9 @@ extern "C" { * everyone's existing deployed numpy test suite passes before * https://github.com/numpy/numpy/issues/22098 is widely available. * - * $ python -m timeit -s 's = * "1"*4300' 'int(s)' + * $ python -m timeit -s 's = "1"*4300' 'int(s)' * 2000 loops, best of 5: 125 usec per loop - * $ python -m timeit -s 's = * "1"*4300; v = int(s)' 'str(v)' + * $ python -m timeit -s 's = "1"*4300; v = int(s)' 'str(v)' * 1000 loops, best of 5: 311 usec per loop * (zen2 cloud VM) * From c49a737eaa4a780b2bf604d06a52ef7eb0258758 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sun, 4 Sep 2022 00:37:30 -0700 Subject: [PATCH 09/13] Change timers if process_time() returns 0. #wasm --- Lib/test/test_int.py | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/Lib/test/test_int.py b/Lib/test/test_int.py index 3b9680d656f58f..47aebd502dc535 100644 --- a/Lib/test/test_int.py +++ b/Lib/test/test_int.py @@ -637,18 +637,22 @@ def test_denial_of_service_prevented_int_to_str(self): """Regression test: ensure we fail before performing O(N**2) work.""" maxdigits = sys.get_int_max_str_digits() assert maxdigits < 50_000, maxdigits # A test prerequisite. - process_time = time.process_time + get_time = time.process_time + if get_time() <= 0: # some platforms like WASM lacks process_time() + get_time = time.monotonic huge_int = int(f'0x{"c"*65_000}', base=16) # 78268 decimal digits. digits = 78_268 with support.adjust_int_max_str_digits(digits): - start = process_time() + start = get_time() huge_decimal = str(huge_int) - seconds_to_convert = process_time() - start + seconds_to_convert = get_time() - start self.assertEqual(len(huge_decimal), digits) # Ensuring that we chose a slow enough conversion to time. # Unlikely any CPU core will ever be faster than the assertion. # It takes 0.10 seconds on a Zen based cloud VM in an opt build. + if seconds_to_convert < 0.005: + raise unittest.SkipTest(f'') self.assertGreater(seconds_to_convert, 0.005, msg="'We're gonna need a bigger boat (int).'") @@ -656,9 +660,9 @@ def test_denial_of_service_prevented_int_to_str(self): # The performant limit check is slightly fuzzy, give it a some room. with support.adjust_int_max_str_digits(int(.995 * digits)): with self.assertRaises(ValueError) as err: - start = process_time() + start = get_time() str(huge_int) - seconds_to_fail_huge = process_time() - start + seconds_to_fail_huge = get_time() - start self.assertIn('conversion', str(err.exception)) self.assertLess(seconds_to_fail_huge, seconds_to_convert/8) @@ -666,10 +670,10 @@ def test_denial_of_service_prevented_int_to_str(self): # in a similarly fast fashion. extra_huge_int = int(f'0x{"c"*500_000}', base=16) # 602060 digits. with self.assertRaises(ValueError) as err: - start = process_time() + start = get_time() # If not limited, 8 seconds said Zen based cloud VM. str(extra_huge_int) - seconds_to_fail_extra_huge = process_time() - start + seconds_to_fail_extra_huge = get_time() - start self.assertIn('conversion', str(err.exception)) self.assertLess(seconds_to_fail_extra_huge, seconds_to_convert/8) @@ -677,13 +681,15 @@ def test_denial_of_service_prevented_str_to_int(self): """Regression test: ensure we fail before performing O(N**2) work.""" maxdigits = sys.get_int_max_str_digits() assert maxdigits < 100_000, maxdigits # A test prerequisite. - process_time = time.process_time + get_time = time.process_time + if get_time() <= 0: # some platforms like WASM lacks process_time() + get_time = time.monotonic huge = '8'*200_000 with support.adjust_int_max_str_digits(200_000): - start = process_time() + start = get_time() int(huge) - seconds_to_convert = process_time() - start + seconds_to_convert = get_time() - start # Ensuring that we chose a slow enough conversion to time. # Unlikely any CPU core will ever be faster than the assertion. # It takes 0.25 seconds on a Zen based cloud VM in an opt build. @@ -692,9 +698,9 @@ def test_denial_of_service_prevented_str_to_int(self): with support.adjust_int_max_str_digits(200_000 - 1): with self.assertRaises(ValueError) as err: - start = process_time() + start = get_time() int(huge) - seconds_to_fail_huge = process_time() - start + seconds_to_fail_huge = get_time() - start self.assertIn('conversion', str(err.exception)) self.assertLess(seconds_to_fail_huge, seconds_to_convert/8) @@ -702,10 +708,10 @@ def test_denial_of_service_prevented_str_to_int(self): # in a similarly fast fashion. extra_huge = '7'*1_200_000 with self.assertRaises(ValueError) as err: - start = process_time() + start = get_time() # If not limited, 8 seconds said Zen based cloud VM. int(extra_huge) - seconds_to_fail_extra_huge = process_time() - start + seconds_to_fail_extra_huge = get_time() - start self.assertIn('conversion', str(err.exception)) self.assertLess(seconds_to_fail_extra_huge, seconds_to_convert/8) From 730915df6b236d1937466c724cc53c79009ef532 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sun, 4 Sep 2022 01:58:50 -0700 Subject: [PATCH 10/13] Update comment to suggest reading the PR for detail --- Objects/longobject.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Objects/longobject.c b/Objects/longobject.c index c825bbaa7c601b..cb52f82687af28 100644 --- a/Objects/longobject.c +++ b/Objects/longobject.c @@ -1759,8 +1759,11 @@ long_to_decimal_string_internal(PyObject *aa, size_a = Py_ABS(Py_SIZE(a)); negative = Py_SIZE(a) < 0; - /* quick and dirty pre-check for overflowing the digit limit, - based on the inequality 10/3 >= log2(10) */ + /* quick and dirty pre-check for overflowing the decimal digit limit, + based on the inequality 10/3 >= log2(10) + + explanation in https://github.com/python/cpython/pull/96537 + */ if (size_a >= 10 * _PY_LONG_MAX_STR_DIGITS_THRESHOLD / (3 * PyLong_SHIFT) + 2) { PyInterpreterState *interp = _PyInterpreterState_GET(); From 56f08c28c33bfa035ccb1b76599b262b6150cc0e Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sun, 4 Sep 2022 02:22:13 -0700 Subject: [PATCH 11/13] Minor comment typo fix (restart CI). --- Lib/test/test_int.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_int.py b/Lib/test/test_int.py index 47aebd502dc535..ea27df462dc2c2 100644 --- a/Lib/test/test_int.py +++ b/Lib/test/test_int.py @@ -638,7 +638,7 @@ def test_denial_of_service_prevented_int_to_str(self): maxdigits = sys.get_int_max_str_digits() assert maxdigits < 50_000, maxdigits # A test prerequisite. get_time = time.process_time - if get_time() <= 0: # some platforms like WASM lacks process_time() + if get_time() <= 0: # some platforms like WASM lack process_time() get_time = time.monotonic huge_int = int(f'0x{"c"*65_000}', base=16) # 78268 decimal digits. @@ -682,7 +682,7 @@ def test_denial_of_service_prevented_str_to_int(self): maxdigits = sys.get_int_max_str_digits() assert maxdigits < 100_000, maxdigits # A test prerequisite. get_time = time.process_time - if get_time() <= 0: # some platforms like WASM lacks process_time() + if get_time() <= 0: # some platforms like WASM lack process_time() get_time = time.monotonic huge = '8'*200_000 From 4dae3e0494bc45b386b243f53b8d19bfb9e29d47 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sun, 4 Sep 2022 02:38:02 -0700 Subject: [PATCH 12/13] cleanup the test. skip rather than fail if we find unexpectedly high performance. --- Lib/test/test_int.py | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_int.py b/Lib/test/test_int.py index ea27df462dc2c2..800c0b006cdc6b 100644 --- a/Lib/test/test_int.py +++ b/Lib/test/test_int.py @@ -648,13 +648,11 @@ def test_denial_of_service_prevented_int_to_str(self): huge_decimal = str(huge_int) seconds_to_convert = get_time() - start self.assertEqual(len(huge_decimal), digits) - # Ensuring that we chose a slow enough conversion to time. - # Unlikely any CPU core will ever be faster than the assertion. - # It takes 0.10 seconds on a Zen based cloud VM in an opt build. + # Ensuring that we chose a slow enough conversion to measure. + # It takes 0.1 seconds on a Zen based cloud VM in an opt build. if seconds_to_convert < 0.005: - raise unittest.SkipTest(f'') - self.assertGreater(seconds_to_convert, 0.005, - msg="'We're gonna need a bigger boat (int).'") + raise unittest.SkipTest('"slow" conversion took only ' + f'{seconds_to_convert} seconds.') # We test with the limit almost at the size needed to check performance. # The performant limit check is slightly fuzzy, give it a some room. @@ -685,18 +683,19 @@ def test_denial_of_service_prevented_str_to_int(self): if get_time() <= 0: # some platforms like WASM lack process_time() get_time = time.monotonic - huge = '8'*200_000 - with support.adjust_int_max_str_digits(200_000): + digits = 133700 + huge = '8'*digits + with support.adjust_int_max_str_digits(digits): start = get_time() int(huge) seconds_to_convert = get_time() - start - # Ensuring that we chose a slow enough conversion to time. - # Unlikely any CPU core will ever be faster than the assertion. - # It takes 0.25 seconds on a Zen based cloud VM in an opt build. - self.assertGreater(seconds_to_convert, 0.02, - msg="'We're gonna need a bigger boat (str).'") + # Ensuring that we chose a slow enough conversion to measure. + # It takes 0.1 seconds on a Zen based cloud VM in an opt build. + if seconds_to_convert < 0.005: + raise unittest.SkipTest('"slow" conversion took only ' + f'{seconds_to_convert} seconds.') - with support.adjust_int_max_str_digits(200_000 - 1): + with support.adjust_int_max_str_digits(digits - 1): with self.assertRaises(ValueError) as err: start = get_time() int(huge) @@ -709,7 +708,7 @@ def test_denial_of_service_prevented_str_to_int(self): extra_huge = '7'*1_200_000 with self.assertRaises(ValueError) as err: start = get_time() - # If not limited, 8 seconds said Zen based cloud VM. + # If not limited, 8 seconds in the Zen based cloud VM. int(extra_huge) seconds_to_fail_extra_huge = get_time() - start self.assertIn('conversion', str(err.exception)) From adb17840c496a0497fff5d07688d26d60cbfe629 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith [Google LLC]" Date: Sun, 4 Sep 2022 02:58:14 -0700 Subject: [PATCH 13/13] Add Mark's name to the NEWS entry. --- .../Security/2022-08-07-16-53-38.gh-issue-95778.ch010gps.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Security/2022-08-07-16-53-38.gh-issue-95778.ch010gps.rst b/Misc/NEWS.d/next/Security/2022-08-07-16-53-38.gh-issue-95778.ch010gps.rst index ea3b85d632e083..8eb8a34884dced 100644 --- a/Misc/NEWS.d/next/Security/2022-08-07-16-53-38.gh-issue-95778.ch010gps.rst +++ b/Misc/NEWS.d/next/Security/2022-08-07-16-53-38.gh-issue-95778.ch010gps.rst @@ -11,4 +11,4 @@ limitation ` documentation. The default limit is 4300 digits in string form. Patch by Gregory P. Smith [Google] and Christian Heimes [Red Hat] with feedback -from Victor Stinner, Thomas Wouters, Steve Dower, and Ned Deily. +from Victor Stinner, Thomas Wouters, Steve Dower, Ned Deily, and Mark Dickinson.