From aab01e3524d966dca6e72c718a2c71403a14e47c Mon Sep 17 00:00:00 2001 From: Pablo Galindo Salgado Date: Tue, 27 Sep 2022 23:23:42 +0100 Subject: gh-96670: Raise SyntaxError when parsing NULL bytes (#97594) --- Doc/whatsnew/3.12.rst | 6 +++++ Include/cpython/fileobject.h | 1 + Lib/test/test_ast.py | 4 +++ Lib/test/test_builtin.py | 3 +-- Lib/test/test_cmd_line_script.py | 12 +++++++++ Lib/test/test_compile.py | 8 +++--- .../2022-09-27-11-59-13.gh-issue-96670.XrBBit.rst | 2 ++ Objects/fileobject.c | 29 ++++++++++++++-------- Parser/tokenizer.c | 19 +++++++++++--- Python/pythonrun.c | 2 +- 10 files changed, 65 insertions(+), 21 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-09-27-11-59-13.gh-issue-96670.XrBBit.rst diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 3fbc7b6..2a31160 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -86,6 +86,12 @@ Other Language Changes * :class:`memoryview` now supports the half-float type (the "e" format code). (Contributed by Dong-hee Na and Antoine Pitrou in :gh:`90751`.) +* The parser now raises :exc:`SyntaxError` when parsing source code containing + null bytes. (Contributed by Pablo Galindo in :gh:`96670`.) + +* :func:`ast.parse` now raises :exc:`SyntaxError` instead of :exc:`ValueError` + when parsing source code containing null bytes. (Contributed by Pablo Galindo + in :gh:`96670`.) New Modules =========== diff --git a/Include/cpython/fileobject.h b/Include/cpython/fileobject.h index cff2243..b70ec31 100644 --- a/Include/cpython/fileobject.h +++ b/Include/cpython/fileobject.h @@ -3,6 +3,7 @@ #endif PyAPI_FUNC(char *) Py_UniversalNewlineFgets(char *, int, FILE*, PyObject *); +PyAPI_FUNC(char *) _Py_UniversalNewlineFgetsWithSize(char *, int, FILE*, PyObject *, size_t*); /* The std printer acts as a preliminary sys.stderr until the new io infrastructure is in place. */ diff --git a/Lib/test/test_ast.py b/Lib/test/test_ast.py index c97d161..9a7df28 100644 --- a/Lib/test/test_ast.py +++ b/Lib/test/test_ast.py @@ -844,6 +844,10 @@ class AST_Tests(unittest.TestCase): check_limit("a", "[0]") check_limit("a", "*a") + def test_null_bytes(self): + with self.assertRaises(SyntaxError, + msg="source code string cannot contain null bytes"): + ast.parse("a\0b") class ASTHelpers_Test(unittest.TestCase): maxDiff = None diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 8c9c1e5..31e5063 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -334,11 +334,10 @@ class BuiltinTest(unittest.TestCase): self.assertRaises(TypeError, compile) self.assertRaises(ValueError, compile, 'print(42)\n', '', 'badmode') self.assertRaises(ValueError, compile, 'print(42)\n', '', 'single', 0xff) - self.assertRaises(ValueError, compile, chr(0), 'f', 'exec') self.assertRaises(TypeError, compile, 'pass', '?', 'exec', mode='eval', source='0', filename='tmp') compile('print("\xe5")\n', '', 'exec') - self.assertRaises(ValueError, compile, chr(0), 'f', 'exec') + self.assertRaises(SyntaxError, compile, chr(0), 'f', 'exec') self.assertRaises(ValueError, compile, str('a = 1'), 'f', 'bad') # test the optimize argument diff --git a/Lib/test/test_cmd_line_script.py b/Lib/test/test_cmd_line_script.py index 9e98edf..1ee3acd 100644 --- a/Lib/test/test_cmd_line_script.py +++ b/Lib/test/test_cmd_line_script.py @@ -657,6 +657,18 @@ class CmdLineTest(unittest.TestCase): ], ) + def test_syntaxerror_null_bytes(self): + script = "x = '\0' nothing to see here\n';import os;os.system('echo pwnd')\n" + with os_helper.temp_dir() as script_dir: + script_name = _make_test_script(script_dir, 'script', script) + exitcode, stdout, stderr = assert_python_failure(script_name) + self.assertEqual( + stderr.splitlines()[-2:], + [ b" x = '", + b'SyntaxError: source code cannot contain null bytes' + ], + ) + def test_consistent_sys_path_for_direct_execution(self): # This test case ensures that the following all give the same # sys.path configuration: diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py index 4b35cce..7c55c71 100644 --- a/Lib/test/test_compile.py +++ b/Lib/test/test_compile.py @@ -544,7 +544,7 @@ if 1: with open(fn, "wb") as fp: fp.write(src) res = script_helper.run_python_until_end(fn)[0] - self.assertIn(b"Non-UTF-8", res.err) + self.assertIn(b"source code cannot contain null bytes", res.err) def test_yet_more_evil_still_undecodable(self): # Issue #25388 @@ -554,7 +554,7 @@ if 1: with open(fn, "wb") as fp: fp.write(src) res = script_helper.run_python_until_end(fn)[0] - self.assertIn(b"Non-UTF-8", res.err) + self.assertIn(b"source code cannot contain null bytes", res.err) @support.cpython_only @unittest.skipIf(support.is_wasi, "exhausts limited stack on WASI") @@ -591,9 +591,9 @@ if 1: def test_null_terminated(self): # The source code is null-terminated internally, but bytes-like # objects are accepted, which could be not terminated. - with self.assertRaisesRegex(ValueError, "cannot contain null"): + with self.assertRaisesRegex(SyntaxError, "cannot contain null"): compile("123\x00", "", "eval") - with self.assertRaisesRegex(ValueError, "cannot contain null"): + with self.assertRaisesRegex(SyntaxError, "cannot contain null"): compile(memoryview(b"123\x00"), "", "eval") code = compile(memoryview(b"123\x00")[1:-1], "", "eval") self.assertEqual(eval(code), 23) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-09-27-11-59-13.gh-issue-96670.XrBBit.rst b/Misc/NEWS.d/next/Core and Builtins/2022-09-27-11-59-13.gh-issue-96670.XrBBit.rst new file mode 100644 index 0000000..ba8e2bb --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-09-27-11-59-13.gh-issue-96670.XrBBit.rst @@ -0,0 +1,2 @@ +The parser now raises :exc:`SyntaxError` when parsing source code containing +null bytes. Patch by Pablo Galindo diff --git a/Objects/fileobject.c b/Objects/fileobject.c index cbc5741..ab67cd2 100644 --- a/Objects/fileobject.c +++ b/Objects/fileobject.c @@ -230,16 +230,8 @@ _PyLong_FileDescriptor_Converter(PyObject *o, void *ptr) return 1; } -/* -** Py_UniversalNewlineFgets is an fgets variation that understands -** all of \r, \n and \r\n conventions. -** The stream should be opened in binary mode. -** The fobj parameter exists solely for legacy reasons and must be NULL. -** Note that we need no error handling: fgets() treats error and eof -** identically. -*/ char * -Py_UniversalNewlineFgets(char *buf, int n, FILE *stream, PyObject *fobj) +_Py_UniversalNewlineFgetsWithSize(char *buf, int n, FILE *stream, PyObject *fobj, size_t* size) { char *p = buf; int c; @@ -265,11 +257,28 @@ Py_UniversalNewlineFgets(char *buf, int n, FILE *stream, PyObject *fobj) } FUNLOCKFILE(stream); *p = '\0'; - if (p == buf) + if (p == buf) { return NULL; + } + *size = p - buf; return buf; } +/* +** Py_UniversalNewlineFgets is an fgets variation that understands +** all of \r, \n and \r\n conventions. +** The stream should be opened in binary mode. +** The fobj parameter exists solely for legacy reasons and must be NULL. +** Note that we need no error handling: fgets() treats error and eof +** identically. +*/ + +char * +Py_UniversalNewlineFgets(char *buf, int n, FILE *stream, PyObject *fobj) { + size_t size; + return _Py_UniversalNewlineFgetsWithSize(buf, n, stream, fobj, &size); +} + /* **************************** std printer **************************** * The stdprinter is used during the boot strapping phase as a preliminary * file like object for sys.stderr. diff --git a/Parser/tokenizer.c b/Parser/tokenizer.c index b3b1185..3c37fd9 100644 --- a/Parser/tokenizer.c +++ b/Parser/tokenizer.c @@ -378,6 +378,11 @@ tok_reserve_buf(struct tok_state *tok, Py_ssize_t size) return 1; } +static inline int +contains_null_bytes(const char* str, size_t size) { + return memchr(str, 0, size) != NULL; +} + static int tok_readline_recode(struct tok_state *tok) { PyObject *line; @@ -829,9 +834,9 @@ tok_readline_raw(struct tok_state *tok) if (!tok_reserve_buf(tok, BUFSIZ)) { return 0; } - char *line = Py_UniversalNewlineFgets(tok->inp, - (int)(tok->end - tok->inp), - tok->fp, NULL); + int n_chars = (int)(tok->end - tok->inp); + size_t line_size = 0; + char *line = _Py_UniversalNewlineFgetsWithSize(tok->inp, n_chars, tok->fp, NULL, &line_size); if (line == NULL) { return 1; } @@ -839,7 +844,7 @@ tok_readline_raw(struct tok_state *tok) tok_concatenate_interactive_new_line(tok, line) == -1) { return 0; } - tok->inp = strchr(tok->inp, '\0'); + tok->inp += line_size; if (tok->inp == tok->buf) { return 0; } @@ -1075,6 +1080,12 @@ tok_nextc(struct tok_state *tok) return EOF; } tok->line_start = tok->cur; + + if (contains_null_bytes(tok->line_start, tok->inp - tok->line_start)) { + syntaxerror(tok, "source code cannot contain null bytes"); + tok->cur = tok->inp; + return EOF; + } } Py_UNREACHABLE(); } diff --git a/Python/pythonrun.c b/Python/pythonrun.c index 202df58..acb1330 100644 --- a/Python/pythonrun.c +++ b/Python/pythonrun.c @@ -1858,7 +1858,7 @@ _Py_SourceAsString(PyObject *cmd, const char *funcname, const char *what, PyComp } if (strlen(str) != (size_t)size) { - PyErr_SetString(PyExc_ValueError, + PyErr_SetString(PyExc_SyntaxError, "source code string cannot contain null bytes"); Py_CLEAR(*cmd_copy); return NULL; -- cgit v0.12