From 35dbad02f84b244283358dcc5e74e5f51ad4bab5 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Wed, 12 Dec 2018 22:57:38 -0800 Subject: [PATCH 1/3] clone command line that should be passed to CreateProcessW --- .../2018-12-12-22-52-34.bpo-31446.l--Fjz.rst | 2 ++ Modules/_winapi.c | 24 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Library/2018-12-12-22-52-34.bpo-31446.l--Fjz.rst diff --git a/Misc/NEWS.d/next/Library/2018-12-12-22-52-34.bpo-31446.l--Fjz.rst b/Misc/NEWS.d/next/Library/2018-12-12-22-52-34.bpo-31446.l--Fjz.rst new file mode 100644 index 00000000000000..741263f16bb4d5 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-12-12-22-52-34.bpo-31446.l--Fjz.rst @@ -0,0 +1,2 @@ +Copy command line that was passed to CreateProcessW since this function can +change the content of the input buffer. diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 44788e5992a0dd..0ea46d8d0c6e5e 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -1008,6 +1008,9 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, STARTUPINFOEXW si; PyObject *environment = NULL; wchar_t *wenvironment; + Py_ssize_t command_line_len; + wchar_t *command_line_copy = NULL; + wchar_t *command_line_to_use = command_line; AttributeList attribute_list = {0}; ZeroMemory(&si, sizeof(si)); @@ -1043,9 +1046,27 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, si.lpAttributeList = attribute_list.attribute_list; + // copy command line since CreateProcessW can change the content of the buffer + // The maximum length of this string is 32,768 characters, + // including the Unicode terminating null character + // if command line that was passed exceeded this range + // CreateProcessW will report an error so there is no point to clone the buffer + // and we can use original string. Otherwise copy the string. + command_line_len = wcslen(command_line); + if (command_line_len < 32768) { + command_line_copy = PyMem_New(wchar_t, command_line_len + 1); + if (command_line_copy == NULL) { + PyErr_NoMemory(); + goto cleanup; + } + wcsncpy(command_line_copy, command_line, command_line_len); + command_line_copy[command_line_len] = L'\0'; + command_line_to_use = command_line_copy; + } + Py_BEGIN_ALLOW_THREADS result = CreateProcessW(application_name, - command_line, + command_line_to_use, NULL, NULL, inherit_handles, @@ -1069,6 +1090,7 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, pi.dwThreadId); cleanup: + PyMem_Free(command_line_copy); Py_XDECREF(environment); freeattributelist(&attribute_list); From daaef205ceab6e49c72b0eda7c2d0376082ebbda Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 13 Dec 2018 11:09:27 -0800 Subject: [PATCH 2/3] use PyUnicode_AsWideCharString instead of explicit alloc\copy --- Modules/_winapi.c | 29 +++++++---------------------- Modules/clinic/_winapi.c.h | 8 ++++---- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index 0ea46d8d0c6e5e..aa558d7cc49784 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -975,7 +975,7 @@ getattributelist(PyObject *obj, const char *name, AttributeList *attribute_list) _winapi.CreateProcess application_name: Py_UNICODE(accept={str, NoneType}) - command_line: Py_UNICODE(accept={str, NoneType}) + command_line: unicode proc_attrs: object Ignored internally, can be None. thread_attrs: object @@ -995,12 +995,12 @@ process ID, and thread ID. static PyObject * _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, - Py_UNICODE *command_line, PyObject *proc_attrs, + PyObject *command_line, PyObject *proc_attrs, PyObject *thread_attrs, BOOL inherit_handles, DWORD creation_flags, PyObject *env_mapping, Py_UNICODE *current_directory, PyObject *startup_info) -/*[clinic end generated code: output=4652a33aff4b0ae1 input=4a43b05038d639bb]*/ +/*[clinic end generated code: output=2ecaab46a05e3123 input=14cbeab1bed18c3a]*/ { PyObject *ret = NULL; BOOL result; @@ -1008,9 +1008,7 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, STARTUPINFOEXW si; PyObject *environment = NULL; wchar_t *wenvironment; - Py_ssize_t command_line_len; wchar_t *command_line_copy = NULL; - wchar_t *command_line_to_use = command_line; AttributeList attribute_list = {0}; ZeroMemory(&si, sizeof(si)); @@ -1046,27 +1044,14 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, si.lpAttributeList = attribute_list.attribute_list; - // copy command line since CreateProcessW can change the content of the buffer - // The maximum length of this string is 32,768 characters, - // including the Unicode terminating null character - // if command line that was passed exceeded this range - // CreateProcessW will report an error so there is no point to clone the buffer - // and we can use original string. Otherwise copy the string. - command_line_len = wcslen(command_line); - if (command_line_len < 32768) { - command_line_copy = PyMem_New(wchar_t, command_line_len + 1); - if (command_line_copy == NULL) { - PyErr_NoMemory(); - goto cleanup; - } - wcsncpy(command_line_copy, command_line, command_line_len); - command_line_copy[command_line_len] = L'\0'; - command_line_to_use = command_line_copy; + command_line_copy = PyUnicode_AsWideCharString(command_line, NULL); + if (command_line_copy == NULL) { + goto cleanup; } Py_BEGIN_ALLOW_THREADS result = CreateProcessW(application_name, - command_line_to_use, + command_line_copy, NULL, NULL, inherit_handles, diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h index f82b46d7381cc2..5eafe156aeb3e6 100644 --- a/Modules/clinic/_winapi.c.h +++ b/Modules/clinic/_winapi.c.h @@ -299,7 +299,7 @@ PyDoc_STRVAR(_winapi_CreateProcess__doc__, static PyObject * _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, - Py_UNICODE *command_line, PyObject *proc_attrs, + PyObject *command_line, PyObject *proc_attrs, PyObject *thread_attrs, BOOL inherit_handles, DWORD creation_flags, PyObject *env_mapping, Py_UNICODE *current_directory, @@ -310,7 +310,7 @@ _winapi_CreateProcess(PyObject *module, PyObject *const *args, Py_ssize_t nargs) { PyObject *return_value = NULL; Py_UNICODE *application_name; - Py_UNICODE *command_line; + PyObject *command_line; PyObject *proc_attrs; PyObject *thread_attrs; BOOL inherit_handles; @@ -319,7 +319,7 @@ _winapi_CreateProcess(PyObject *module, PyObject *const *args, Py_ssize_t nargs) Py_UNICODE *current_directory; PyObject *startup_info; - if (!_PyArg_ParseStack(args, nargs, "ZZOOikOZO:CreateProcess", + if (!_PyArg_ParseStack(args, nargs, "ZUOOikOZO:CreateProcess", &application_name, &command_line, &proc_attrs, &thread_attrs, &inherit_handles, &creation_flags, &env_mapping, ¤t_directory, &startup_info)) { goto exit; } @@ -941,4 +941,4 @@ _winapi_GetFileType(PyObject *module, PyObject *const *args, Py_ssize_t nargs, P exit: return return_value; } -/*[clinic end generated code: output=915dd640329de0c0 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1d55dc299b855905 input=a9049054013a1b77]*/ From 9502eaf33dc19cff9ae25da8de16988c2e58f579 Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 13 Dec 2018 21:53:30 -0800 Subject: [PATCH 3/3] handle str/None as command line argument types --- Modules/_winapi.c | 19 ++++++++++++++----- Modules/clinic/_winapi.c.h | 6 ++++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Modules/_winapi.c b/Modules/_winapi.c index aa558d7cc49784..852e0a7d7b4857 100644 --- a/Modules/_winapi.c +++ b/Modules/_winapi.c @@ -975,7 +975,8 @@ getattributelist(PyObject *obj, const char *name, AttributeList *attribute_list) _winapi.CreateProcess application_name: Py_UNICODE(accept={str, NoneType}) - command_line: unicode + command_line: object + Can be str or None proc_attrs: object Ignored internally, can be None. thread_attrs: object @@ -1000,7 +1001,7 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, DWORD creation_flags, PyObject *env_mapping, Py_UNICODE *current_directory, PyObject *startup_info) -/*[clinic end generated code: output=2ecaab46a05e3123 input=14cbeab1bed18c3a]*/ +/*[clinic end generated code: output=2ecaab46a05e3123 input=42ac293eaea03fc4]*/ { PyObject *ret = NULL; BOOL result; @@ -1043,12 +1044,20 @@ _winapi_CreateProcess_impl(PyObject *module, Py_UNICODE *application_name, goto cleanup; si.lpAttributeList = attribute_list.attribute_list; - - command_line_copy = PyUnicode_AsWideCharString(command_line, NULL); - if (command_line_copy == NULL) { + if (PyUnicode_Check(command_line)) { + command_line_copy = PyUnicode_AsWideCharString(command_line, NULL); + if (command_line_copy == NULL) { + goto cleanup; + } + } + else if (command_line != Py_None) { + PyErr_Format(PyExc_TypeError, + "CreateProcess() argument 2 must be str or None, not %s", + Py_TYPE(command_line)->tp_name); goto cleanup; } + Py_BEGIN_ALLOW_THREADS result = CreateProcessW(application_name, command_line_copy, diff --git a/Modules/clinic/_winapi.c.h b/Modules/clinic/_winapi.c.h index 5eafe156aeb3e6..241c18e5a25279 100644 --- a/Modules/clinic/_winapi.c.h +++ b/Modules/clinic/_winapi.c.h @@ -286,6 +286,8 @@ PyDoc_STRVAR(_winapi_CreateProcess__doc__, "\n" "Create a new process and its primary thread.\n" "\n" +" command_line\n" +" Can be str or None\n" " proc_attrs\n" " Ignored internally, can be None.\n" " thread_attrs\n" @@ -319,7 +321,7 @@ _winapi_CreateProcess(PyObject *module, PyObject *const *args, Py_ssize_t nargs) Py_UNICODE *current_directory; PyObject *startup_info; - if (!_PyArg_ParseStack(args, nargs, "ZUOOikOZO:CreateProcess", + if (!_PyArg_ParseStack(args, nargs, "ZOOOikOZO:CreateProcess", &application_name, &command_line, &proc_attrs, &thread_attrs, &inherit_handles, &creation_flags, &env_mapping, ¤t_directory, &startup_info)) { goto exit; } @@ -941,4 +943,4 @@ _winapi_GetFileType(PyObject *module, PyObject *const *args, Py_ssize_t nargs, P exit: return return_value; } -/*[clinic end generated code: output=1d55dc299b855905 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1568ad4bd625f2af input=a9049054013a1b77]*/