From fd105a8d1b2e72393bbedc907ca34d57e63e7277 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 3 Mar 2025 20:03:07 +0300 Subject: [PATCH 1/4] gh-130775: Validate negative locations in `ast` --- Lib/test/test_ast/test_ast.py | 21 +++++++++++++++++++ ...-03-03-20-02-45.gh-issue-130775.fEM6T-.rst | 2 ++ Python/assemble.c | 1 + Python/ast.c | 8 +++++++ 4 files changed, 32 insertions(+) create mode 100644 Misc/NEWS.d/next/Core_and_Builtins/2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index 6e1458facafc30..20a9953ac822d0 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -204,6 +204,27 @@ def test_compilation_of_ast_nodes_with_default_end_position_values(self): # Check that compilation doesn't crash. Note: this may crash explicitly only on debug mode. compile(tree, "", "exec") + def test_compilation_of_ast_nodes_with_negative_position_values(self): + # See https://github.com/python/cpython/issues/130775 + alias = ast.alias(name='traceback', lineno=0, col_offset=0) + for attrs in ( + {'lineno': -2, 'col_offset': 0}, + {'lineno': 0, 'col_offset': -2}, + {'lineno': 0, 'col_offset': -2, 'end_col_offset': -2}, + {'lineno': -2, 'end_lineno': -2, 'col_offset': 0}, + ): + with self.subTest(attrs=attrs): + tree = ast.Module(body=[ + ast.Import(names=[alias], **attrs) + ], type_ignores=[]) + + # It used to crash on this step: + with self.assertRaisesRegex( + ValueError, + 'AST node has invalid location', + ): + compile(tree, "", "exec") + def test_slice(self): slc = ast.parse("x[::]").body[0].value.slice self.assertIsNone(slc.upper) diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst new file mode 100644 index 00000000000000..4c28ae11977e04 --- /dev/null +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst @@ -0,0 +1,2 @@ +Validate negative :mod:`ast` locations, we only allow ``-1`` as a special +value. diff --git a/Python/assemble.c b/Python/assemble.c index 6dcac332f076d8..dfa5ad1e57e868 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -297,6 +297,7 @@ write_location_info_entry(struct assembler* a, location loc, int isize) int line_delta = loc.lineno - a->a_lineno; int column = loc.col_offset; int end_column = loc.end_col_offset; + // Values are validated in `VALIDATE_POSITIONS` in `ast.c`: assert(column >= -1); assert(end_column >= -1); if (column < 0 || end_column < 0) { diff --git a/Python/ast.c b/Python/ast.c index 597df5b63b5f39..0e45e45c6060c8 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -40,6 +40,14 @@ static int validate_typeparam(type_param_ty); node->col_offset, node->end_col_offset, node->lineno, node->end_lineno); \ return 0; \ } \ + if (node->lineno < -1 || node->end_lineno < -1 || \ + node->col_offset < -1 || node->end_col_offset < -1) { \ + PyErr_Format(PyExc_ValueError, \ + "AST node has invalid location (%d, %d, %d, %d)", \ + node->lineno, node->end_lineno, \ + node->col_offset, node->end_col_offset); \ + return 0; \ + } \ if (node->lineno == node->end_lineno && node->col_offset > node->end_col_offset) { \ PyErr_Format(PyExc_ValueError, \ "line %d, column %d-%d is not a valid range", \ From 9d2037ebdeab7fd95feeda865264bb68f2098f64 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Mon, 3 Mar 2025 21:50:35 +0300 Subject: [PATCH 2/4] Update Python/ast.c Co-authored-by: Victor Stinner --- Python/ast.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Python/ast.c b/Python/ast.c index 0e45e45c6060c8..fc3df40d01b13d 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -43,7 +43,8 @@ static int validate_typeparam(type_param_ty); if (node->lineno < -1 || node->end_lineno < -1 || \ node->col_offset < -1 || node->end_col_offset < -1) { \ PyErr_Format(PyExc_ValueError, \ - "AST node has invalid location (%d, %d, %d, %d)", \ + "AST node has invalid location: lineno=%d, end_lineno=%d, " \ + "col_offset=%d, end_col_offset=%d", \ node->lineno, node->end_lineno, \ node->col_offset, node->end_col_offset); \ return 0; \ From 1427c571e6aa47ad5452837c3e995ede0d254ee7 Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 13 Mar 2025 19:24:51 +0300 Subject: [PATCH 3/4] Address reviews --- Lib/test/test_ast/test_ast.py | 11 +++++------ .../2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst | 3 +-- Python/assemble.c | 5 +---- Python/ast.c | 9 --------- 4 files changed, 7 insertions(+), 21 deletions(-) diff --git a/Lib/test/test_ast/test_ast.py b/Lib/test/test_ast/test_ast.py index 20a9953ac822d0..d5d623e1b5d118 100644 --- a/Lib/test/test_ast/test_ast.py +++ b/Lib/test/test_ast/test_ast.py @@ -204,7 +204,7 @@ def test_compilation_of_ast_nodes_with_default_end_position_values(self): # Check that compilation doesn't crash. Note: this may crash explicitly only on debug mode. compile(tree, "", "exec") - def test_compilation_of_ast_nodes_with_negative_position_values(self): + def test_negative_locations_for_compile(self): # See https://github.com/python/cpython/issues/130775 alias = ast.alias(name='traceback', lineno=0, col_offset=0) for attrs in ( @@ -219,11 +219,10 @@ def test_compilation_of_ast_nodes_with_negative_position_values(self): ], type_ignores=[]) # It used to crash on this step: - with self.assertRaisesRegex( - ValueError, - 'AST node has invalid location', - ): - compile(tree, "", "exec") + compile(tree, "", "exec") + + # This also must not crash: + ast.parse(tree, optimize=2) def test_slice(self): slc = ast.parse("x[::]").body[0].value.slice diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst b/Misc/NEWS.d/next/Core_and_Builtins/2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst index 4c28ae11977e04..53408cd427596c 100644 --- a/Misc/NEWS.d/next/Core_and_Builtins/2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst +++ b/Misc/NEWS.d/next/Core_and_Builtins/2025-03-03-20-02-45.gh-issue-130775.fEM6T-.rst @@ -1,2 +1 @@ -Validate negative :mod:`ast` locations, we only allow ``-1`` as a special -value. +Do not crash on negative ``column`` and ``end_column`` in :mod:`ast` locations. diff --git a/Python/assemble.c b/Python/assemble.c index dfa5ad1e57e868..ad1c23223a21a1 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -297,11 +297,8 @@ write_location_info_entry(struct assembler* a, location loc, int isize) int line_delta = loc.lineno - a->a_lineno; int column = loc.col_offset; int end_column = loc.end_col_offset; - // Values are validated in `VALIDATE_POSITIONS` in `ast.c`: - assert(column >= -1); - assert(end_column >= -1); if (column < 0 || end_column < 0) { - if (loc.end_lineno == loc.lineno || loc.end_lineno == -1) { + if (loc.end_lineno == loc.lineno || loc.end_lineno < 0) { write_location_info_no_column(a, isize, line_delta); a->a_lineno = loc.lineno; return SUCCESS; diff --git a/Python/ast.c b/Python/ast.c index fc3df40d01b13d..597df5b63b5f39 100644 --- a/Python/ast.c +++ b/Python/ast.c @@ -40,15 +40,6 @@ static int validate_typeparam(type_param_ty); node->col_offset, node->end_col_offset, node->lineno, node->end_lineno); \ return 0; \ } \ - if (node->lineno < -1 || node->end_lineno < -1 || \ - node->col_offset < -1 || node->end_col_offset < -1) { \ - PyErr_Format(PyExc_ValueError, \ - "AST node has invalid location: lineno=%d, end_lineno=%d, " \ - "col_offset=%d, end_col_offset=%d", \ - node->lineno, node->end_lineno, \ - node->col_offset, node->end_col_offset); \ - return 0; \ - } \ if (node->lineno == node->end_lineno && node->col_offset > node->end_col_offset) { \ PyErr_Format(PyExc_ValueError, \ "line %d, column %d-%d is not a valid range", \ From 8f3e1a9f15d04cfa163baf3af5e00be6b43623fa Mon Sep 17 00:00:00 2001 From: sobolevn Date: Thu, 13 Mar 2025 20:01:45 +0300 Subject: [PATCH 4/4] Fix tests --- Python/assemble.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/assemble.c b/Python/assemble.c index 84ca594b0e184b..8b5d031d9a4e1a 100644 --- a/Python/assemble.c +++ b/Python/assemble.c @@ -290,8 +290,7 @@ write_location_info_entry(struct assembler* a, location loc, int isize) assert(len > THEORETICAL_MAX_ENTRY_SIZE); RETURN_IF_ERROR(_PyBytes_Resize(&a->a_linetable, len*2)); } - if (loc.lineno < 0) { - assert(loc.lineno == NO_LOCATION.lineno); + if (loc.lineno == NO_LOCATION.lineno) { write_location_info_none(a, isize); return SUCCESS; }