From 117152f5db69f9c79b2f8ea1a9746fec5de6c13f Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Sat, 15 Feb 2025 01:34:53 +0000 Subject: [PATCH 1/2] gh-130139: always check ast node type in ast.parse() with ast input --- Doc/whatsnew/3.14.rst | 4 +++ Include/internal/pycore_ast.h | 1 + Lib/test/test_ast/test_ast.py | 6 ++++ Lib/test/test_unparse.py | 4 +-- ...-02-15-01-37-47.gh-issue-130139.gntc7B.rst | 2 ++ Parser/asdl_c.py | 35 +++++++++++++------ Python/Python-ast.c | 34 ++++++++++++------ Python/bltinmodule.c | 3 ++ 8 files changed, 65 insertions(+), 24 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index e40b597ee52157..9d2b763f6a0753 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -333,6 +333,10 @@ ast * The ``repr()`` output for AST nodes now includes more information. (Contributed by Tomas R in :gh:`116022`.) +* ast.parse(), when called with an AST as input, now always verifies + that the root node type is appropriate. + (Contributed by Irit Katriel in :gh:`130139`.) + calendar -------- diff --git a/Include/internal/pycore_ast.h b/Include/internal/pycore_ast.h index f5bf1205a82be9..69abc3536e312a 100644 --- a/Include/internal/pycore_ast.h +++ b/Include/internal/pycore_ast.h @@ -907,6 +907,7 @@ type_param_ty _PyAST_TypeVarTuple(identifier name, expr_ty default_value, int PyObject* PyAST_mod2obj(mod_ty t); +int PyAst_CheckMode(PyObject *ast, int mode); mod_ty PyAST_obj2mod(PyObject* ast, PyArena* arena, int mode); int PyAST_Check(PyObject* obj); diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index a438c8e81e4fd1..461f9e85f9d43e 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -131,6 +131,12 @@ def test_ast_validation(self): tree = ast.parse(snippet) compile(tree, '', 'exec') + def test_parse_invalid_ast(self): + # see gh-130139 + for optval in (-1, 0, 1, 2): + self.assertRaises(TypeError, ast.parse, ast.Constant(42), + optimize=optval) + def test_optimization_levels__debug__(self): cases = [(-1, '__debug__'), (0, '__debug__'), (1, False), (2, False)] for (optval, expected) in cases: diff --git a/Lib/test/test_unparse.py b/Lib/test/test_unparse.py index f6c4f1f3f6476a..f45a651c7ccb5d 100644 --- a/Lib/test/test_unparse.py +++ b/Lib/test/test_unparse.py @@ -422,9 +422,9 @@ def test_docstrings(self): self.check_ast_roundtrip(f"'''{docstring}'''") def test_constant_tuples(self): - self.check_src_roundtrip(ast.Constant(value=(1,), kind=None), "(1,)") + self.check_src_roundtrip(ast.Module([ast.Constant(value=(1,))]), "(1,)") self.check_src_roundtrip( - ast.Constant(value=(1, 2, 3), kind=None), "(1, 2, 3)" + ast.Module([ast.Constant(value=(1, 2, 3))]), "(1, 2, 3)" ) def test_function_type(self): diff --git a/Misc/NEWS.d/next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst b/Misc/NEWS.d/next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst new file mode 100644 index 00000000000000..6a316f31a8304a --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst @@ -0,0 +1,2 @@ +Fix bug where ``ast.parse`` did not error on AST input which is not of the +correct type, when called with optimize=False. diff --git a/Parser/asdl_c.py b/Parser/asdl_c.py index 7b2df738119115..b2a5dd6792dead 100755 --- a/Parser/asdl_c.py +++ b/Parser/asdl_c.py @@ -2166,18 +2166,13 @@ class PartingShots(StaticVisitor): } /* mode is 0 for "exec", 1 for "eval" and 2 for "single" input */ -mod_ty PyAST_obj2mod(PyObject* ast, PyArena* arena, int mode) +int PyAst_CheckMode(PyObject *ast, int mode) { const char * const req_name[] = {"Module", "Expression", "Interactive"}; - int isinstance; - - if (PySys_Audit("compile", "OO", ast, Py_None) < 0) { - return NULL; - } struct ast_state *state = get_ast_state(); if (state == NULL) { - return NULL; + return -1; } PyObject *req_type[3]; @@ -2186,13 +2181,30 @@ class PartingShots(StaticVisitor): req_type[2] = state->Interactive_type; assert(0 <= mode && mode <= 2); - - isinstance = PyObject_IsInstance(ast, req_type[mode]); - if (isinstance == -1) - return NULL; + int isinstance = PyObject_IsInstance(ast, req_type[mode]); + if (isinstance == -1) { + return -1; + } if (!isinstance) { PyErr_Format(PyExc_TypeError, "expected %s node, got %.400s", req_name[mode], _PyType_Name(Py_TYPE(ast))); + return -1; + } + return 0; +} + +mod_ty PyAST_obj2mod(PyObject* ast, PyArena* arena, int mode) +{ + if (PySys_Audit("compile", "OO", ast, Py_None) < 0) { + return NULL; + } + + struct ast_state *state = get_ast_state(); + if (state == NULL) { + return NULL; + } + + if (PyAst_CheckMode(ast, mode) < 0) { return NULL; } @@ -2356,6 +2368,7 @@ def write_header(mod, metadata, f): f.write(textwrap.dedent(""" PyObject* PyAST_mod2obj(mod_ty t); + int PyAst_CheckMode(PyObject *ast, int mode); mod_ty PyAST_obj2mod(PyObject* ast, PyArena* arena, int mode); int PyAST_Check(PyObject* obj); diff --git a/Python/Python-ast.c b/Python/Python-ast.c index 7038e3c92ab8f0..4adf72a8537a51 100644 --- a/Python/Python-ast.c +++ b/Python/Python-ast.c @@ -18161,18 +18161,13 @@ PyObject* PyAST_mod2obj(mod_ty t) } /* mode is 0 for "exec", 1 for "eval" and 2 for "single" input */ -mod_ty PyAST_obj2mod(PyObject* ast, PyArena* arena, int mode) +int PyAst_CheckMode(PyObject *ast, int mode) { const char * const req_name[] = {"Module", "Expression", "Interactive"}; - int isinstance; - - if (PySys_Audit("compile", "OO", ast, Py_None) < 0) { - return NULL; - } struct ast_state *state = get_ast_state(); if (state == NULL) { - return NULL; + return -1; } PyObject *req_type[3]; @@ -18181,13 +18176,30 @@ mod_ty PyAST_obj2mod(PyObject* ast, PyArena* arena, int mode) req_type[2] = state->Interactive_type; assert(0 <= mode && mode <= 2); - - isinstance = PyObject_IsInstance(ast, req_type[mode]); - if (isinstance == -1) - return NULL; + int isinstance = PyObject_IsInstance(ast, req_type[mode]); + if (isinstance == -1) { + return -1; + } if (!isinstance) { PyErr_Format(PyExc_TypeError, "expected %s node, got %.400s", req_name[mode], _PyType_Name(Py_TYPE(ast))); + return -1; + } + return 0; +} + +mod_ty PyAST_obj2mod(PyObject* ast, PyArena* arena, int mode) +{ + if (PySys_Audit("compile", "OO", ast, Py_None) < 0) { + return NULL; + } + + struct ast_state *state = get_ast_state(); + if (state == NULL) { + return NULL; + } + + if (PyAst_CheckMode(ast, mode) < 0) { return NULL; } diff --git a/Python/bltinmodule.c b/Python/bltinmodule.c index 46a6fd9a8ef017..a7243baa64c2a8 100644 --- a/Python/bltinmodule.c +++ b/Python/bltinmodule.c @@ -835,6 +835,9 @@ builtin_compile_impl(PyObject *module, PyObject *source, PyObject *filename, goto error; if (is_ast) { if ((flags & PyCF_OPTIMIZED_AST) == PyCF_ONLY_AST) { + if (PyAst_CheckMode(source, compile_mode) < 0) { + goto error; + } // return an un-optimized AST result = Py_NewRef(source); } From 75307d5f75f29314433a69d8390d6d37c250fe40 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Sat, 15 Feb 2025 09:14:25 +0000 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Kirill Podoprigora --- Doc/whatsnew/3.14.rst | 2 +- .../next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index 9d2b763f6a0753..a271140815e97a 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -333,7 +333,7 @@ ast * The ``repr()`` output for AST nodes now includes more information. (Contributed by Tomas R in :gh:`116022`.) -* ast.parse(), when called with an AST as input, now always verifies +* :func:`ast.parse`, when called with an AST as input, now always verifies that the root node type is appropriate. (Contributed by Irit Katriel in :gh:`130139`.) diff --git a/Misc/NEWS.d/next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst b/Misc/NEWS.d/next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst index 6a316f31a8304a..5cb3bf141a77d7 100644 --- a/Misc/NEWS.d/next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst +++ b/Misc/NEWS.d/next/Library/2025-02-15-01-37-47.gh-issue-130139.gntc7B.rst @@ -1,2 +1,2 @@ -Fix bug where ``ast.parse`` did not error on AST input which is not of the +Fix bug where :func:`ast.parse` did not error on AST input which is not of the correct type, when called with optimize=False.