From dff8b2f15f94d5c40677b68c3771ec5c096c7d8c Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 18 Jul 2023 13:02:02 +0300 Subject: [PATCH 1/5] gh-106844: Fix null-bytes handling in `LCMapStringEx` in `_winapi` --- Lib/test/test_ntpath.py | 1 + ...-07-18-13-01-26.gh-issue-106844.mci4xO.rst | 2 + Modules/_winapi.c | 40 +++++++++++++++---- Modules/clinic/_winapi.c.h | 19 ++++----- 4 files changed, 42 insertions(+), 20 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index 538d758624c9d6..bff7a02a0ac81c 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -1036,6 +1036,7 @@ def test_path_normcase(self): self._check_function(self.path.normcase) if sys.platform == 'win32': self.assertEqual(ntpath.normcase('\u03a9\u2126'), 'ωΩ') + self.assertEqual(ntpath.normcase('abc\x00def'), 'abcdef') def test_path_isabs(self): self._check_function(self.path.isabs) diff --git a/Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst b/Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst new file mode 100644 index 00000000000000..177f95da1deaa3 --- /dev/null +++ b/Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst @@ -0,0 +1,2 @@ +Fix null-bytes handling in ``LCMapStringEx`` which affects +:func:`ntpath.normcase`. diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 313c12a34c6725..f523c8f2b5fd92 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1537,42 +1537,66 @@ _winapi_PeekNamedPipe_impl(PyObject *module, HANDLE handle, int size) /*[clinic input] _winapi.LCMapStringEx - locale: LPCWSTR + locale: unicode flags: DWORD - src: LPCWSTR + src: unicode [clinic start generated code]*/ static PyObject * -_winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags, - LPCWSTR src) -/*[clinic end generated code: output=cf4713d80e2b47c9 input=9fe26f95d5ab0001]*/ +_winapi_LCMapStringEx_impl(PyObject *module, PyObject *locale, DWORD flags, + PyObject *src) +/*[clinic end generated code: output=8ea4c9d85a4a1f23 input=2fa6ebc92591731b]*/ { if (flags & (LCMAP_SORTHANDLE | LCMAP_HASH | LCMAP_BYTEREV | LCMAP_SORTKEY)) { return PyErr_Format(PyExc_ValueError, "unsupported flags"); } - int dest_size = LCMapStringEx(locale, flags, src, -1, NULL, 0, + wchar_t *locale_ = PyUnicode_AsWideCharString(locale, NULL); + if (!locale_) { + return NULL; + } + Py_ssize_t srcLenAsSsize; + int srcLen; + wchar_t *src_ = PyUnicode_AsWideCharString(src, &srcLenAsSsize); + if (!src_) { + PyMem_Free(locale_); + return NULL; + } + srcLen = (int)srcLenAsSsize; + if (srcLen != srcLenAsSsize) { + srcLen = -1; + } + + int dest_size = LCMapStringEx(locale_, flags, src_, srcLen, NULL, 0, NULL, NULL, 0); if (dest_size == 0) { + PyMem_Free(locale_); + PyMem_Free(src_); return PyErr_SetFromWindowsErr(0); } wchar_t* dest = PyMem_NEW(wchar_t, dest_size); if (dest == NULL) { + PyMem_Free(locale_); + PyMem_Free(src_); return PyErr_NoMemory(); } - int nmapped = LCMapStringEx(locale, flags, src, -1, dest, dest_size, + int nmapped = LCMapStringEx(locale_, flags, src_, srcLen, dest, dest_size, NULL, NULL, 0); if (nmapped == 0) { DWORD error = GetLastError(); + PyMem_Free(locale_); + PyMem_Free(src_); PyMem_DEL(dest); return PyErr_SetFromWindowsErr(error); } - PyObject *ret = PyUnicode_FromWideChar(dest, dest_size - 1); + PyObject *ret = PyUnicode_FromWideChar(dest, dest_size); + PyMem_Free(locale_); + PyMem_Free(src_); PyMem_DEL(dest); return ret; diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h index 8f46b8f1095e98..98b1006adb7583 100644 --- a/Modules/clinic/_winapi.c.h +++ b/Modules/clinic/_winapi.c.h @@ -883,8 +883,8 @@ PyDoc_STRVAR(_winapi_LCMapStringEx__doc__, {"LCMapStringEx", _PyCFunction_CAST(_winapi_LCMapStringEx), METH_FASTCALL|METH_KEYWORDS, _winapi_LCMapStringEx__doc__}, static PyObject * -_winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags, - LPCWSTR src); +_winapi_LCMapStringEx_impl(PyObject *module, PyObject *locale, DWORD flags, + PyObject *src); static PyObject * _winapi_LCMapStringEx(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) @@ -911,26 +911,21 @@ _winapi_LCMapStringEx(PyObject *module, PyObject *const *args, Py_ssize_t nargs, static const char * const _keywords[] = {"locale", "flags", "src", NULL}; static _PyArg_Parser _parser = { .keywords = _keywords, - .format = "O&kO&:LCMapStringEx", + .format = "UkU:LCMapStringEx", .kwtuple = KWTUPLE, }; #undef KWTUPLE - LPCWSTR locale = NULL; + PyObject *locale; DWORD flags; - LPCWSTR src = NULL; + PyObject *src; if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, - _PyUnicode_WideCharString_Converter, &locale, &flags, _PyUnicode_WideCharString_Converter, &src)) { + &locale, &flags, &src)) { goto exit; } return_value = _winapi_LCMapStringEx_impl(module, locale, flags, src); exit: - /* Cleanup for locale */ - PyMem_Free((void *)locale); - /* Cleanup for src */ - PyMem_Free((void *)src); - return return_value; } @@ -1480,4 +1475,4 @@ _winapi_CopyFile2(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyO return return_value; } -/*[clinic end generated code: output=f32fe6ecdbffd74d input=a9049054013a1b77]*/ +/*[clinic end generated code: output=6dab4295cebee066 input=a9049054013a1b77]*/ From 46be70ba601d3df0495e227bb4b0ca69aaed6d8b Mon Sep 17 00:00:00 2001 From: sobolevn Date: Tue, 18 Jul 2023 14:06:13 +0300 Subject: [PATCH 2/5] Fix CI --- Lib/test/test_ntpath.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ntpath.py b/Lib/test/test_ntpath.py index bff7a02a0ac81c..78e1cb582512b0 100644 --- a/Lib/test/test_ntpath.py +++ b/Lib/test/test_ntpath.py @@ -1036,7 +1036,7 @@ def test_path_normcase(self): self._check_function(self.path.normcase) if sys.platform == 'win32': self.assertEqual(ntpath.normcase('\u03a9\u2126'), 'ωΩ') - self.assertEqual(ntpath.normcase('abc\x00def'), 'abcdef') + self.assertEqual(ntpath.normcase('abc\x00def'), 'abc\x00def') def test_path_isabs(self): self._check_function(self.path.isabs) From 13ce79c19b3a330dcc19d3e3d52e54654d89fe26 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 10 Aug 2023 11:26:14 +0300 Subject: [PATCH 3/5] Keep simpler code for the locale parameter. --- Modules/_winapi.c | 19 +++++-------------- Modules/clinic/_winapi.c.h | 13 ++++++++----- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index f523c8f2b5fd92..2fadc2afaf4e2c 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1537,31 +1537,26 @@ _winapi_PeekNamedPipe_impl(PyObject *module, HANDLE handle, int size) /*[clinic input] _winapi.LCMapStringEx - locale: unicode + locale: LPCWSTR flags: DWORD src: unicode [clinic start generated code]*/ static PyObject * -_winapi_LCMapStringEx_impl(PyObject *module, PyObject *locale, DWORD flags, +_winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags, PyObject *src) -/*[clinic end generated code: output=8ea4c9d85a4a1f23 input=2fa6ebc92591731b]*/ +/*[clinic end generated code: output=b90e6b26e028ff0a input=3e3dcd9b8164012f]*/ { if (flags & (LCMAP_SORTHANDLE | LCMAP_HASH | LCMAP_BYTEREV | LCMAP_SORTKEY)) { return PyErr_Format(PyExc_ValueError, "unsupported flags"); } - wchar_t *locale_ = PyUnicode_AsWideCharString(locale, NULL); - if (!locale_) { - return NULL; - } Py_ssize_t srcLenAsSsize; int srcLen; wchar_t *src_ = PyUnicode_AsWideCharString(src, &srcLenAsSsize); if (!src_) { - PyMem_Free(locale_); return NULL; } srcLen = (int)srcLenAsSsize; @@ -1569,33 +1564,29 @@ _winapi_LCMapStringEx_impl(PyObject *module, PyObject *locale, DWORD flags, srcLen = -1; } - int dest_size = LCMapStringEx(locale_, flags, src_, srcLen, NULL, 0, + int dest_size = LCMapStringEx(locale, flags, src_, srcLen, NULL, 0, NULL, NULL, 0); if (dest_size == 0) { - PyMem_Free(locale_); PyMem_Free(src_); return PyErr_SetFromWindowsErr(0); } wchar_t* dest = PyMem_NEW(wchar_t, dest_size); if (dest == NULL) { - PyMem_Free(locale_); PyMem_Free(src_); return PyErr_NoMemory(); } - int nmapped = LCMapStringEx(locale_, flags, src_, srcLen, dest, dest_size, + int nmapped = LCMapStringEx(locale, flags, src_, srcLen, dest, dest_size, NULL, NULL, 0); if (nmapped == 0) { DWORD error = GetLastError(); - PyMem_Free(locale_); PyMem_Free(src_); PyMem_DEL(dest); return PyErr_SetFromWindowsErr(error); } PyObject *ret = PyUnicode_FromWideChar(dest, dest_size); - PyMem_Free(locale_); PyMem_Free(src_); PyMem_DEL(dest); diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h index 98b1006adb7583..35ac053547121c 100644 --- a/Modules/clinic/_winapi.c.h +++ b/Modules/clinic/_winapi.c.h @@ -883,7 +883,7 @@ PyDoc_STRVAR(_winapi_LCMapStringEx__doc__, {"LCMapStringEx", _PyCFunction_CAST(_winapi_LCMapStringEx), METH_FASTCALL|METH_KEYWORDS, _winapi_LCMapStringEx__doc__}, static PyObject * -_winapi_LCMapStringEx_impl(PyObject *module, PyObject *locale, DWORD flags, +_winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags, PyObject *src); static PyObject * @@ -911,21 +911,24 @@ _winapi_LCMapStringEx(PyObject *module, PyObject *const *args, Py_ssize_t nargs, static const char * const _keywords[] = {"locale", "flags", "src", NULL}; static _PyArg_Parser _parser = { .keywords = _keywords, - .format = "UkU:LCMapStringEx", + .format = "O&kU:LCMapStringEx", .kwtuple = KWTUPLE, }; #undef KWTUPLE - PyObject *locale; + LPCWSTR locale = NULL; DWORD flags; PyObject *src; if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, - &locale, &flags, &src)) { + _PyUnicode_WideCharString_Converter, &locale, &flags, &src)) { goto exit; } return_value = _winapi_LCMapStringEx_impl(module, locale, flags, src); exit: + /* Cleanup for locale */ + PyMem_Free((void *)locale); + return return_value; } @@ -1475,4 +1478,4 @@ _winapi_CopyFile2(PyObject *module, PyObject *const *args, Py_ssize_t nargs, PyO return return_value; } -/*[clinic end generated code: output=6dab4295cebee066 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=ff91ab5cae8961dd input=a9049054013a1b77]*/ From 88d4193d3ae3903b975ecfe132c60e639d7acf8a Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 10 Aug 2023 11:36:12 +0300 Subject: [PATCH 4/5] Handle large strings containing the null character. --- Modules/_winapi.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 2fadc2afaf4e2c..aa75ab01d10fc0 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1553,18 +1553,18 @@ _winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags, return PyErr_Format(PyExc_ValueError, "unsupported flags"); } - Py_ssize_t srcLenAsSsize; - int srcLen; - wchar_t *src_ = PyUnicode_AsWideCharString(src, &srcLenAsSsize); + Py_ssize_t src_size; + wchar_t *src_ = PyUnicode_AsWideCharString(src, &src_size); if (!src_) { return NULL; } - srcLen = (int)srcLenAsSsize; - if (srcLen != srcLenAsSsize) { - srcLen = -1; + if (src_size > INT_MAX) { + PyMem_Free(src_); + PyErr_SetString(PyExc_OverflowError, "input string is too long"); + return NULL; } - int dest_size = LCMapStringEx(locale, flags, src_, srcLen, NULL, 0, + int dest_size = LCMapStringEx(locale, flags, src_, (int)src_size, NULL, 0, NULL, NULL, 0); if (dest_size == 0) { PyMem_Free(src_); @@ -1577,7 +1577,7 @@ _winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags, return PyErr_NoMemory(); } - int nmapped = LCMapStringEx(locale, flags, src_, srcLen, dest, dest_size, + int nmapped = LCMapStringEx(locale, flags, src_, (int)src_size, dest, dest_size, NULL, NULL, 0); if (nmapped == 0) { DWORD error = GetLastError(); From 26161c381509f2a397bfb59f69ba3488dfdfe224 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Fri, 11 Aug 2023 19:28:14 +0300 Subject: [PATCH 5/5] Fix other issues. --- .../2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst | 3 +-- Modules/_winapi.c | 9 +++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst b/Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst index 177f95da1deaa3..1fdf162ef4ecdd 100644 --- a/Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst +++ b/Misc/NEWS.d/next/Windows/2023-07-18-13-01-26.gh-issue-106844.mci4xO.rst @@ -1,2 +1 @@ -Fix null-bytes handling in ``LCMapStringEx`` which affects -:func:`ntpath.normcase`. +Fix integer overflow and truncating by the null character in :func:`!_winapi.LCMapStringEx` which affects :func:`ntpath.normcase`. diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 6aab8462411e25..eec33499b983fe 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1566,9 +1566,10 @@ _winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags, int dest_size = LCMapStringEx(locale, flags, src_, (int)src_size, NULL, 0, NULL, NULL, 0); - if (dest_size == 0) { + if (dest_size <= 0) { + DWORD error = GetLastError(); PyMem_Free(src_); - return PyErr_SetFromWindowsErr(0); + return PyErr_SetFromWindowsErr(error); } wchar_t* dest = PyMem_NEW(wchar_t, dest_size); @@ -1579,15 +1580,15 @@ _winapi_LCMapStringEx_impl(PyObject *module, LPCWSTR locale, DWORD flags, int nmapped = LCMapStringEx(locale, flags, src_, (int)src_size, dest, dest_size, NULL, NULL, 0); - if (nmapped == 0) { + if (nmapped <= 0) { DWORD error = GetLastError(); PyMem_Free(src_); PyMem_DEL(dest); return PyErr_SetFromWindowsErr(error); } - PyObject *ret = PyUnicode_FromWideChar(dest, dest_size); PyMem_Free(src_); + PyObject *ret = PyUnicode_FromWideChar(dest, nmapped); PyMem_DEL(dest); return ret;