From 54ba940abc2fabb94fede46dfad80f8ac15632a3 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 28 Jun 2017 10:31:00 +0300 Subject: [3.5] bpo-13617: Reject embedded null characters in wchar* strings. (GH-2302) (#2463) Based on patch by Victor Stinner. Add private C API function _PyUnicode_AsUnicode() which is similar to PyUnicode_AsUnicode(), but checks for null characters.. (cherry picked from commit f7eae0adfcd4c50034281b2c69f461b43b68db84) --- Include/unicodeobject.h | 10 +++++++--- Lib/ctypes/test/test_loading.py | 2 ++ Lib/test/test_builtin.py | 6 ++++++ Lib/test/test_curses.py | 11 ++++++++++- Lib/test/test_grp.py | 2 ++ Lib/test/test_imp.py | 4 ++++ Lib/test/test_locale.py | 5 +++++ Lib/test/test_time.py | 4 ++++ Lib/test/test_winsound.py | 4 ++++ Modules/_ctypes/callproc.c | 7 ++++--- Modules/_cursesmodule.c | 9 +++++++++ Modules/_localemodule.c | 5 +++++ Modules/grpmodule.c | 1 + Modules/nismodule.c | 1 + Modules/posixmodule.c | 24 +++++++++++++++--------- Modules/pwdmodule.c | 1 + Modules/spwdmodule.c | 1 + Objects/unicodeobject.c | 14 ++++++++++++++ PC/_msi.c | 6 +++++- Python/dynload_win.c | 4 ++-- Python/fileutils.c | 23 +++++++++++++++++------ 21 files changed, 119 insertions(+), 25 deletions(-) diff --git a/Include/unicodeobject.h b/Include/unicodeobject.h index 0e44808..f0ececc 100644 --- a/Include/unicodeobject.h +++ b/Include/unicodeobject.h @@ -764,23 +764,27 @@ PyAPI_FUNC(Py_UCS4*) PyUnicode_AsUCS4( exception set. */ PyAPI_FUNC(Py_UCS4*) PyUnicode_AsUCS4Copy(PyObject *unicode); +#ifndef Py_LIMITED_API /* Return a read-only pointer to the Unicode object's internal Py_UNICODE buffer. If the wchar_t/Py_UNICODE representation is not yet available, this function will calculate it. */ -#ifndef Py_LIMITED_API PyAPI_FUNC(Py_UNICODE *) PyUnicode_AsUnicode( PyObject *unicode /* Unicode object */ ); -#endif + +/* Similar to PyUnicode_AsUnicode(), but raises a ValueError if the string + contains null characters. */ +PyAPI_FUNC(const Py_UNICODE *) _PyUnicode_AsUnicode( + PyObject *unicode /* Unicode object */ + ); /* Return a read-only pointer to the Unicode object's internal Py_UNICODE buffer and save the length at size. If the wchar_t/Py_UNICODE representation is not yet available, this function will calculate it. */ -#ifndef Py_LIMITED_API PyAPI_FUNC(Py_UNICODE *) PyUnicode_AsUnicodeAndSize( PyObject *unicode, /* Unicode object */ Py_ssize_t *size /* location where to save the length */ diff --git a/Lib/ctypes/test/test_loading.py b/Lib/ctypes/test/test_loading.py index 28468c1..3de457f 100644 --- a/Lib/ctypes/test/test_loading.py +++ b/Lib/ctypes/test/test_loading.py @@ -64,6 +64,8 @@ class LoaderTest(unittest.TestCase): windll["kernel32"].GetModuleHandleW windll.LoadLibrary("kernel32").GetModuleHandleW WinDLL("kernel32").GetModuleHandleW + # embedded null character + self.assertRaises(ValueError, windll.LoadLibrary, "kernel32\0") elif os.name == "ce": windll.coredll.GetModuleHandleW windll["coredll"].GetModuleHandleW diff --git a/Lib/test/test_builtin.py b/Lib/test/test_builtin.py index 0b03340..18e80c7 100644 --- a/Lib/test/test_builtin.py +++ b/Lib/test/test_builtin.py @@ -151,6 +151,8 @@ class BuiltinTest(unittest.TestCase): self.assertRaises(TypeError, __import__, 1, 2, 3, 4) self.assertRaises(ValueError, __import__, '') self.assertRaises(TypeError, __import__, 'sys', name='sys') + # embedded null character + self.assertRaises((ImportError, ValueError), __import__, 'string\x00') def test_abs(self): # int @@ -1002,6 +1004,10 @@ class BuiltinTest(unittest.TestCase): self.assertEqual(fp.read(300), 'XXX'*100) self.assertEqual(fp.read(1000), 'YYY'*100) + # embedded null bytes and characters + self.assertRaises(ValueError, open, 'a\x00b') + self.assertRaises(ValueError, open, b'a\x00b') + def test_open_default_encoding(self): old_environ = dict(os.environ) try: diff --git a/Lib/test/test_curses.py b/Lib/test/test_curses.py index 3d8c50b..0d0b160 100644 --- a/Lib/test/test_curses.py +++ b/Lib/test/test_curses.py @@ -81,7 +81,7 @@ class TestCurses(unittest.TestCase): win2 = curses.newwin(15,15, 5,5) for meth in [stdscr.addch, stdscr.addstr]: - for args in [('a'), ('a', curses.A_BOLD), + for args in [('a',), ('a', curses.A_BOLD), (4,4, 'a'), (5,5, 'a', curses.A_BOLD)]: with self.subTest(meth=meth.__qualname__, args=args): meth(*args) @@ -194,6 +194,15 @@ class TestCurses(unittest.TestCase): self.assertRaises(ValueError, stdscr.instr, -2) self.assertRaises(ValueError, stdscr.instr, 2, 3, -2) + def test_embedded_null_chars(self): + # reject embedded null bytes and characters + stdscr = self.stdscr + for arg in ['a', b'a']: + with self.subTest(arg=arg): + self.assertRaises(ValueError, stdscr.addstr, 'a\0') + self.assertRaises(ValueError, stdscr.addnstr, 'a\0', 1) + self.assertRaises(ValueError, stdscr.insstr, 'a\0') + self.assertRaises(ValueError, stdscr.insnstr, 'a\0', 1) def test_module_funcs(self): "Test module-level functions" diff --git a/Lib/test/test_grp.py b/Lib/test/test_grp.py index 272b086..c218ea3 100644 --- a/Lib/test/test_grp.py +++ b/Lib/test/test_grp.py @@ -50,6 +50,8 @@ class GroupDatabaseTestCase(unittest.TestCase): self.assertRaises(TypeError, grp.getgrgid) self.assertRaises(TypeError, grp.getgrnam) self.assertRaises(TypeError, grp.getgrall, 42) + # embedded null character + self.assertRaises(ValueError, grp.getgrnam, 'a\x00b') # try to get some errors bynames = {} diff --git a/Lib/test/test_imp.py b/Lib/test/test_imp.py index ee9ee1a..433de34 100644 --- a/Lib/test/test_imp.py +++ b/Lib/test/test_imp.py @@ -315,6 +315,10 @@ class ImportTests(unittest.TestCase): loader.get_data(imp.__file__) # File should be closed loader.get_data(imp.__file__) # Will need to create a newly opened file + def test_load_source(self): + with self.assertRaisesRegex(ValueError, 'embedded null'): + imp.load_source(__name__, __file__ + "\0") + class ReloadTests(unittest.TestCase): diff --git a/Lib/test/test_locale.py b/Lib/test/test_locale.py index fae2c3d..8d40a94 100644 --- a/Lib/test/test_locale.py +++ b/Lib/test/test_locale.py @@ -339,9 +339,14 @@ class TestCollation(unittest.TestCase): self.assertLess(locale.strcoll('a', 'b'), 0) self.assertEqual(locale.strcoll('a', 'a'), 0) self.assertGreater(locale.strcoll('b', 'a'), 0) + # embedded null character + self.assertRaises(ValueError, locale.strcoll, 'a\0', 'a') + self.assertRaises(ValueError, locale.strcoll, 'a', 'a\0') def test_strxfrm(self): self.assertLess(locale.strxfrm('a'), locale.strxfrm('b')) + # embedded null character + self.assertRaises(ValueError, locale.strxfrm, 'a\0') class TestEnUSCollation(BaseLocalizedTest, TestCollation): diff --git a/Lib/test/test_time.py b/Lib/test/test_time.py index 76b894e..04962b2 100644 --- a/Lib/test/test_time.py +++ b/Lib/test/test_time.py @@ -114,6 +114,10 @@ class TimeTestCase(unittest.TestCase): except ValueError: self.fail('conversion specifier: %r failed.' % format) + self.assertRaises(TypeError, time.strftime, b'%S', tt) + # embedded null character + self.assertRaises(ValueError, time.strftime, '%S\0', tt) + def _bounds_checking(self, func): # Make sure that strftime() checks the bounds of the various parts # of the time tuple (0 is valid for *all* values). diff --git a/Lib/test/test_winsound.py b/Lib/test/test_winsound.py index 4a8ab7d..caac3c3 100644 --- a/Lib/test/test_winsound.py +++ b/Lib/test/test_winsound.py @@ -87,6 +87,10 @@ class PlaySoundTest(unittest.TestCase): winsound.PlaySound, "none", winsound.SND_ASYNC | winsound.SND_MEMORY ) + self.assertRaises(TypeError, winsound.PlaySound, b"bad", 0) + self.assertRaises(TypeError, winsound.PlaySound, 1, 0) + # embedded null character + self.assertRaises(ValueError, winsound.PlaySound, 'bad\0', 0) def test_aliases(self): aliases = [ diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index 873df8b..70e416b 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -1235,14 +1235,15 @@ The handle may be used to locate exported functions in this\n\ module.\n"; static PyObject *load_library(PyObject *self, PyObject *args) { - WCHAR *name; + const WCHAR *name; PyObject *nameobj; PyObject *ignored; HMODULE hMod; - if (!PyArg_ParseTuple(args, "O|O:LoadLibrary", &nameobj, &ignored)) + + if (!PyArg_ParseTuple(args, "U|O:LoadLibrary", &nameobj, &ignored)) return NULL; - name = PyUnicode_AsUnicode(nameobj); + name = _PyUnicode_AsUnicode(nameobj); if (!name) return NULL; diff --git a/Modules/_cursesmodule.c b/Modules/_cursesmodule.c index 26d2731..0cf17de 100644 --- a/Modules/_cursesmodule.c +++ b/Modules/_cursesmodule.c @@ -342,6 +342,7 @@ static int PyCurses_ConvertToString(PyCursesWindowObject *win, PyObject *obj, PyObject **bytes, wchar_t **wstr) { + char *str; if (PyUnicode_Check(obj)) { #ifdef HAVE_NCURSESW assert (wstr != NULL); @@ -354,12 +355,20 @@ PyCurses_ConvertToString(PyCursesWindowObject *win, PyObject *obj, *bytes = PyUnicode_AsEncodedString(obj, win->encoding, NULL); if (*bytes == NULL) return 0; + /* check for embedded null bytes */ + if (PyBytes_AsStringAndSize(*bytes, &str, NULL) < 0) { + return 0; + } return 1; #endif } else if (PyBytes_Check(obj)) { Py_INCREF(obj); *bytes = obj; + /* check for embedded null bytes */ + if (PyBytes_AsStringAndSize(*bytes, &str, NULL) < 0) { + return 0; + } return 1; } diff --git a/Modules/_localemodule.c b/Modules/_localemodule.c index 4ce71de..39e0c54 100644 --- a/Modules/_localemodule.c +++ b/Modules/_localemodule.c @@ -252,6 +252,11 @@ PyLocale_strxfrm(PyObject* self, PyObject* args) s = PyUnicode_AsWideCharString(str, &n1); if (s == NULL) goto exit; + if (wcslen(s) != (size_t)n1) { + PyErr_SetString(PyExc_ValueError, + "embedded null character"); + goto exit; + } /* assume no change in size, first */ n1 = n1 + 1; diff --git a/Modules/grpmodule.c b/Modules/grpmodule.c index 3a134a0..277fce4 100644 --- a/Modules/grpmodule.c +++ b/Modules/grpmodule.c @@ -140,6 +140,7 @@ grp_getgrnam_impl(PyObject *module, PyObject *name) if ((bytes = PyUnicode_EncodeFSDefault(name)) == NULL) return NULL; + /* check for embedded null bytes */ if (PyBytes_AsStringAndSize(bytes, &name_chars, NULL) == -1) goto out; diff --git a/Modules/nismodule.c b/Modules/nismodule.c index 64eb5db..3942acb 100644 --- a/Modules/nismodule.c +++ b/Modules/nismodule.c @@ -173,6 +173,7 @@ nis_match (PyObject *self, PyObject *args, PyObject *kwdict) return NULL; if ((bkey = PyUnicode_EncodeFSDefault(ukey)) == NULL) return NULL; + /* check for embedded null bytes */ if (PyBytes_AsStringAndSize(bkey, &key, &keylen) == -1) { Py_DECREF(bkey); return NULL; diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 8d32ddf..3dcebf4 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -3821,9 +3821,9 @@ os__getfinalpathname_impl(PyObject *module, PyObject *path) wchar_t *target_path; int result_length; PyObject *result; - wchar_t *path_wchar; + const wchar_t *path_wchar; - path_wchar = PyUnicode_AsUnicode(path); + path_wchar = _PyUnicode_AsUnicode(path); if (path_wchar == NULL) return NULL; @@ -7164,7 +7164,7 @@ exit: static PyObject * win_readlink(PyObject *self, PyObject *args, PyObject *kwargs) { - wchar_t *path; + const wchar_t *path; DWORD n_bytes_returned; DWORD io_result; PyObject *po, *result; @@ -7183,7 +7183,7 @@ win_readlink(PyObject *self, PyObject *args, PyObject *kwargs) )) return NULL; - path = PyUnicode_AsUnicode(po); + path = _PyUnicode_AsUnicode(po); if (path == NULL) return NULL; @@ -9075,7 +9075,8 @@ static PyObject * os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) /*[clinic end generated code: output=d29a567d6b2327d2 input=ba586581c2e6105f]*/ { - wchar_t *env; + const wchar_t *env; + Py_ssize_t size; /* Search from index 1 because on Windows starting '=' is allowed for defining hidden environment variables. */ @@ -9089,16 +9090,21 @@ os_putenv_impl(PyObject *module, PyObject *name, PyObject *value) if (unicode == NULL) { return NULL; } - if (_MAX_ENV < PyUnicode_GET_LENGTH(unicode)) { + + env = PyUnicode_AsUnicodeAndSize(unicode, &size); + if (env == NULL) + goto error; + if (size > _MAX_ENV) { PyErr_Format(PyExc_ValueError, "the environment variable is longer than %u characters", _MAX_ENV); goto error; } - - env = PyUnicode_AsUnicode(unicode); - if (env == NULL) + if (wcslen(env) != (size_t)size) { + PyErr_SetString(PyExc_ValueError, "embedded null character"); goto error; + } + if (_wputenv(env)) { posix_error(); goto error; diff --git a/Modules/pwdmodule.c b/Modules/pwdmodule.c index 7416cf7..1b0d499 100644 --- a/Modules/pwdmodule.c +++ b/Modules/pwdmodule.c @@ -150,6 +150,7 @@ pwd_getpwnam_impl(PyObject *module, PyObject *arg) if ((bytes = PyUnicode_EncodeFSDefault(arg)) == NULL) return NULL; + /* check for embedded null bytes */ if (PyBytes_AsStringAndSize(bytes, &name, NULL) == -1) goto out; if ((p = getpwnam(name)) == NULL) { diff --git a/Modules/spwdmodule.c b/Modules/spwdmodule.c index 4b9f3cd..065f79c 100644 --- a/Modules/spwdmodule.c +++ b/Modules/spwdmodule.c @@ -134,6 +134,7 @@ spwd_getspnam_impl(PyObject *module, PyObject *arg) if ((bytes = PyUnicode_EncodeFSDefault(arg)) == NULL) return NULL; + /* check for embedded null bytes */ if (PyBytes_AsStringAndSize(bytes, &name, NULL) == -1) goto out; if ((p = getspnam(name)) == NULL) { diff --git a/Objects/unicodeobject.c b/Objects/unicodeobject.c index 64375da..66cb4af 100644 --- a/Objects/unicodeobject.c +++ b/Objects/unicodeobject.c @@ -3894,6 +3894,20 @@ PyUnicode_AsUnicode(PyObject *unicode) return PyUnicode_AsUnicodeAndSize(unicode, NULL); } +const Py_UNICODE * +_PyUnicode_AsUnicode(PyObject *unicode) +{ + Py_ssize_t size; + const Py_UNICODE *wstr; + + wstr = PyUnicode_AsUnicodeAndSize(unicode, &size); + if (wstr && wcslen(wstr) != (size_t)size) { + PyErr_SetString(PyExc_ValueError, "embedded null character"); + return NULL; + } + return wstr; +} + Py_ssize_t PyUnicode_GetSize(PyObject *unicode) diff --git a/PC/_msi.c b/PC/_msi.c index 9e7e36d..967b7fa 100644 --- a/PC/_msi.c +++ b/PC/_msi.c @@ -600,8 +600,12 @@ summary_setproperty(msiobj* si, PyObject *args) return NULL; if (PyUnicode_Check(data)) { + const WCHAR *value = _PyUnicode_AsUnicode(data); + if (value == NULL) { + return NULL; + } status = MsiSummaryInfoSetPropertyW(si->h, field, VT_LPSTR, - 0, NULL, PyUnicode_AsUnicode(data)); + 0, NULL, value); } else if (PyLong_CheckExact(data)) { long value = PyLong_AsLong(data); if (value == -1 && PyErr_Occurred()) { diff --git a/Python/dynload_win.c b/Python/dynload_win.c index f2c796e..3f533d1 100644 --- a/Python/dynload_win.c +++ b/Python/dynload_win.c @@ -192,13 +192,13 @@ dl_funcptr _PyImport_FindSharedFuncptrWindows(const char *prefix, { dl_funcptr p; char funcname[258], *import_python; - wchar_t *wpathname; + const wchar_t *wpathname; #ifndef _DEBUG _Py_CheckPython3(); #endif - wpathname = PyUnicode_AsUnicode(pathname); + wpathname = _PyUnicode_AsUnicode(pathname); if (wpathname == NULL) return NULL; diff --git a/Python/fileutils.c b/Python/fileutils.c index 23eed71..5072118 100644 --- a/Python/fileutils.c +++ b/Python/fileutils.c @@ -710,21 +710,32 @@ _Py_stat(PyObject *path, struct stat *statbuf) #ifdef MS_WINDOWS int err; struct _stat wstatbuf; - wchar_t *wpath; + const wchar_t *wpath; - wpath = PyUnicode_AsUnicode(path); + wpath = _PyUnicode_AsUnicode(path); if (wpath == NULL) return -2; + err = _wstat(wpath, &wstatbuf); if (!err) statbuf->st_mode = wstatbuf.st_mode; return err; #else int ret; - PyObject *bytes = PyUnicode_EncodeFSDefault(path); + PyObject *bytes; + char *cpath; + + bytes = PyUnicode_EncodeFSDefault(path); if (bytes == NULL) return -2; - ret = stat(PyBytes_AS_STRING(bytes), statbuf); + + /* check for embedded null bytes */ + if (PyBytes_AsStringAndSize(bytes, &cpath, NULL) == -1) { + Py_DECREF(bytes); + return -2; + } + + ret = stat(cpath, statbuf); Py_DECREF(bytes); return ret; #endif @@ -1083,7 +1094,7 @@ _Py_fopen_obj(PyObject *path, const char *mode) FILE *f; int async_err = 0; #ifdef MS_WINDOWS - wchar_t *wpath; + const wchar_t *wpath; wchar_t wmode[10]; int usize; @@ -1097,7 +1108,7 @@ _Py_fopen_obj(PyObject *path, const char *mode) Py_TYPE(path)); return NULL; } - wpath = PyUnicode_AsUnicode(path); + wpath = _PyUnicode_AsUnicode(path); if (wpath == NULL) return NULL; -- cgit v0.12