From 554507dbf7b32bf6c5e9f2b5c2093fec37bf0474 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 6 Oct 2024 00:48:14 +0200 Subject: [PATCH 1/7] Fix use-after-free in AST repr() --- Parser/asdl_c.py | 4 ++-- Python/Python-ast.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py index ab5fd229cc46ea..ea93821f4e1cf2 100755 --- a/Parser/asdl_c.py +++ b/Parser/asdl_c.py @@ -1604,14 +1604,14 @@ def visitModule(self, mod): value_repr = PyObject_Repr(value); } - Py_DECREF(value); - if (!value_repr) { Py_DECREF(name); Py_DECREF(value); goto error; } + Py_DECREF(value); + if (i > 0) { if (_PyUnicodeWriter_WriteASCIIString(&writer, ", ", 2) < 0) { Py_DECREF(name); diff --git a/Python/Python-ast.c b/Python/Python-ast.c index 4a58c0973d1118..fa4d4260d72975 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -5805,14 +5805,14 @@ ast_repr_max_depth(AST_object *self, int depth) value_repr = PyObject_Repr(value); } - Py_DECREF(value); - if (!value_repr) { Py_DECREF(name); Py_DECREF(value); goto error; } + Py_DECREF(value); + if (i > 0) { if (_PyUnicodeWriter_WriteASCIIString(&writer, ", ", 2) < 0) { Py_DECREF(name); From c83821fa05ce6f840b73061f581af8449ff9e8e3 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 6 Oct 2024 00:49:45 +0200 Subject: [PATCH 2/7] Add news entry --- .../2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst new file mode 100644 index 00000000000000..727c678092a9b7 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst @@ -0,0 +1,2 @@ +Fix a use-after-free bug in the :func:`repr` implementation of +:class:`ast.AST` nodes. From 32efba7fa6bb70da641d0a52709d0e6dbfc71d88 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 6 Oct 2024 12:36:43 +0200 Subject: [PATCH 3/7] Add tests --- Lib/test/test_ast/test_ast.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index f052822cb45273..ded6f3dfb7b974 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -1614,6 +1614,12 @@ def test_literal_eval_syntax_errors(self): (\ \ ''') + def test_literal_eval_large_input_crash(self): + # gh-125010: Fix use-after-free in ast repr() + source = "{0x0" + "e" * 250_000 + "%" + "e" * 250_000 + "1j}" + with self.assertRaisesRegex(ValueError, "Exceeds the limit"): + ast.literal_eval(source) + def test_bad_integer(self): # issue13436: Bad error message with invalid numeric values body = [ast.ImportFrom(module='time', From 49a405d7098686196ab19dc1b3875e0c58027298 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 6 Oct 2024 14:11:23 +0200 Subject: [PATCH 4/7] Add a clarification to the test --- Lib/test/test_ast/test_ast.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index ded6f3dfb7b974..1f2aebbb768dae 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -1616,8 +1616,13 @@ def test_literal_eval_syntax_errors(self): def test_literal_eval_large_input_crash(self): # gh-125010: Fix use-after-free in ast repr() + # Note: Without setting sys.set_int_max_str_digits(0), + # this code throws a 'ValueError: Exceeds the limit (4300 digits)'. + # With sys.set_int_max_str_digits(0), + # this code throws a 'ValueError: malformed node or string on line 1': source = "{0x0" + "e" * 250_000 + "%" + "e" * 250_000 + "1j}" - with self.assertRaisesRegex(ValueError, "Exceeds the limit"): + with self.assertRaisesRegex(ValueError, + r"Exceeds the limit \(4300 digits\)"): ast.literal_eval(source) def test_bad_integer(self): From a44751144ef4debeab39a96607a38ff6b041fe4f Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 6 Oct 2024 16:09:48 +0200 Subject: [PATCH 5/7] Simplify test --- Lib/test/test_ast/test_ast.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index 1f2aebbb768dae..01d2e392302e86 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -789,6 +789,13 @@ def test_repr(self) -> None: with self.subTest(test_input=test): self.assertEqual(repr(ast.parse(test)), snapshot) + def test_repr_large_input_crash(self): + # gh-125010: Fix use-after-free in ast repr() + source = "0x0" + "e" * 10_000 + with self.assertRaisesRegex(ValueError, + r"Exceeds the limit \(\d+ digits\)"): + repr(ast.Constant(value=eval(source))) + class CopyTests(unittest.TestCase): """Test copying and pickling AST nodes.""" @@ -1614,17 +1621,6 @@ def test_literal_eval_syntax_errors(self): (\ \ ''') - def test_literal_eval_large_input_crash(self): - # gh-125010: Fix use-after-free in ast repr() - # Note: Without setting sys.set_int_max_str_digits(0), - # this code throws a 'ValueError: Exceeds the limit (4300 digits)'. - # With sys.set_int_max_str_digits(0), - # this code throws a 'ValueError: malformed node or string on line 1': - source = "{0x0" + "e" * 250_000 + "%" + "e" * 250_000 + "1j}" - with self.assertRaisesRegex(ValueError, - r"Exceeds the limit \(4300 digits\)"): - ast.literal_eval(source) - def test_bad_integer(self): # issue13436: Bad error message with invalid numeric values body = [ast.ImportFrom(module='time', From 3dd380c1b9a5d3959b3846b5efa6af785472f442 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 6 Oct 2024 17:19:51 +0200 Subject: [PATCH 6/7] Simplify decref logic --- Parser/asdl_c.py | 5 ++--- Python/Python-ast.c | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py index ea93821f4e1cf2..f50c28afcfe205 100755 --- a/Parser/asdl_c.py +++ b/Parser/asdl_c.py @@ -1604,14 +1604,13 @@ def visitModule(self, mod): value_repr = PyObject_Repr(value); } + Py_DECREF(value); + if (!value_repr) { Py_DECREF(name); - Py_DECREF(value); goto error; } - Py_DECREF(value); - if (i > 0) { if (_PyUnicodeWriter_WriteASCIIString(&writer, ", ", 2) < 0) { Py_DECREF(name); diff --git a/Python/Python-ast.c b/Python/Python-ast.c index fa4d4260d72975..89c52b9dc73cac 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -5805,14 +5805,13 @@ ast_repr_max_depth(AST_object *self, int depth) value_repr = PyObject_Repr(value); } + Py_DECREF(value); + if (!value_repr) { Py_DECREF(name); - Py_DECREF(value); goto error; } - Py_DECREF(value); - if (i > 0) { if (_PyUnicodeWriter_WriteASCIIString(&writer, ", ", 2) < 0) { Py_DECREF(name); From 12b30984ac80bd3c84339200e28619fb5a56ff1b Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sun, 6 Oct 2024 18:19:42 +0200 Subject: [PATCH 7/7] Remove news entry --- .../2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst deleted file mode 100644 index 727c678092a9b7..00000000000000 --- a/Misc/NEWS.d/next/Core_and_Builtins/2024-10-06-00-49-37.gh-issue-125010.gGlhaj.rst +++ /dev/null @@ -1,2 +0,0 @@ -Fix a use-after-free bug in the :func:`repr` implementation of -:class:`ast.AST` nodes.