classification
Title: Ensure that the result of PyUnicode_AsWideCharString() doesn't contain null characters if size is not returned
Type: enhancement Stage: resolved
Components: Interpreter Core, Unicode Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: ezio.melotti, serhiy.storchaka, steve.dower, vstinner
Priority: normal Keywords:

Created on 2017-06-20 04:38 by serhiy.storchaka, last changed 2017-06-27 19:06 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 2285 merged serhiy.storchaka, 2017-06-20 04:45
PR 2443 merged serhiy.storchaka, 2017-06-27 14:39
PR 2448 merged serhiy.storchaka, 2017-06-27 18:15
Messages (7)
msg296401 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-20 04:38
The second parameter of the PyUnicode_AsWideCharString() function

    wchar_t* PyUnicode_AsWideCharString(PyObject *unicode, Py_ssize_t *size)

is a pointer to Py_ssize_t. The size of created wchar_t array is saved on this pointer if it is not NULL. If NULL is passed as the second argument, the only way to determine the size of the wchar_t string is using wcslen(). But if the string contains the null characters, it looks truncated for wcslen() and other C API functions.

Reliable code should always pass the non-NULL second argument and check that wcslen() is equal to the returned string size. See for example the code in Modules/_io/winconsoleio.c. Passing NULL as the second argument is unsafe. But most code doesn't do such check (see all other usages of PyUnicode_AsWideCharString(..., NULL)). And this check complicates the callers code.

I propose to make the check for null characters inside of PyUnicode_AsWideCharString() if NULL is passes as the second argument. This will fix all unsafe usages of PyUnicode_AsWideCharString() and allow to simplify the reliable code.

This issue fixes the part of issue13617.
msg296514 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-21 04:43
This change needs changing one ctypes test, and can break third-party tests or even a code. That is why it is targeted only for 3.7. I'm going to backport the change as a private function for using in CPython internally since this can fix vulnerabilities.
msg296755 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-24 06:31
Could anyone please make a review of this PR? Especially the documentation part. This PR is a part of a set of PRs that fix potential vulnerabilities (issue13617, issue30730, and yet few issues planned).
msg297031 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-27 13:03
New changeset e613e6add5f07ff6aad5802924596b631b707d2a by Serhiy Storchaka in branch 'master':
bpo-30708: Check for null characters in PyUnicode_AsWideCharString(). (#2285)
https://github.com/python/cpython/commit/e613e6add5f07ff6aad5802924596b631b707d2a
msg297062 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-27 18:09
New changeset 0edffa3073b551ffeca34952529e7b292f1bd350 by Serhiy Storchaka in branch '3.6':
[3.6] bpo-30708: Check for null characters in PyUnicode_AsWideCharString(). (GH-2285) (#2443)
https://github.com/python/cpython/commit/0edffa3073b551ffeca34952529e7b292f1bd350
msg297066 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-27 18:52
New changeset 94b169fe48bc7ea76b926823885d1b12c2c381fa by Serhiy Storchaka in branch '3.5':
[3.5] bpo-30708: Add private C API function _PyUnicode_AsWideCharString(). (GH-2285) (GH-2443)  (#2448)
https://github.com/python/cpython/commit/94b169fe48bc7ea76b926823885d1b12c2c381fa
msg297069 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-27 19:06
Wrong commit message in 3.6. Should be the same as in 3.5.

This functionality was backported as a private function _PyUnicode_AsWideCharString().
History
Date User Action Args
2017-06-27 19:06:14serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg297069

stage: patch review -> resolved
2017-06-27 18:52:12serhiy.storchakasetmessages: + msg297066
2017-06-27 18:15:24serhiy.storchakasetpull_requests: + pull_request2505
2017-06-27 18:09:01serhiy.storchakasetmessages: + msg297062
2017-06-27 14:39:50serhiy.storchakasetpull_requests: + pull_request2497
2017-06-27 13:03:16serhiy.storchakasetmessages: + msg297031
2017-06-24 06:31:49serhiy.storchakasetmessages: + msg296755
2017-06-21 04:43:11serhiy.storchakasetmessages: + msg296514
2017-06-20 15:42:20serhiy.storchakalinkissue13617 dependencies
2017-06-20 04:45:09serhiy.storchakasetpull_requests: + pull_request2333
2017-06-20 04:38:10serhiy.storchakacreate