GeezerVison is showing github.com — Reject embedded null characters in wchar* strings · Issue #57826 · python/cpython
Reject embedded null characters in wchar* strings #57826Description
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-06-28.08:22:41.245>
created_at = <Date 2011-12-17.03:53:41.721>
labels = ['type-security', '3.7', 'library', 'expert-unicode']
title = 'Reject embedded null characters in wchar* strings'
updated_at = <Date 2017-06-28.09:24:05.176>
user = 'https://github.com/vstinner' bugs.python.org fields:
activity = <Date 2017-06-28.09:24:05.176>
actor = 'vstinner'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-06-28.08:22:41.245>
closer = 'serhiy.storchaka'
components = ['Library (Lib)', 'Unicode']
creation = <Date 2011-12-17.03:53:41.721>
creator = 'vstinner'
dependencies = ['30708']
files = ['23984', '24034', '24037', '24041']
hgrepos = []
issue_num = 13617
keywords = ['patch']
message_count = 22.0
messages = ['149656', '149657', '149783', '149787', '149791', '149794', '149802', '173147', '221844', '226414', '236699', '296241', '296332', '296461', '296512', '297132', '297155', '297156', '297158', '297161', '297162', '297165']
nosy_count = 6.0
nosy_names = ['vstinner', 'ezio.melotti', 'benhoyt', 'python-dev', 'arnaudc', 'serhiy.storchaka']
pr_nums = ['2302', '2462', '2463', '2464']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue13617'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7'] Activityvstinner commented on Dec 17, 2011 The curses module (only since Python 3.3), locale.strcoll(), locale.strxfrm(), time.strftime() and imp.NullImporter() (only on Windows) accept embedded null characters, whereas they convert the Unicode string to a wide character (wchar_t*) string.
The problem is that the null character truncates the string. Example:
>>> locale.strxfrm('a')
'a'
>>> locale.strxfrm('a\0b')
'a'Attached patch fixes these functions. I wrote the patch for Python 3.3. vstinner commented on Dec 17, 2011 PyUnicode_AsWideCharString() documentation should also warn about this issue. arnaudc commented on Dec 18, 2011 Here is a patch for the documentation.
I added warnings for, PyUnicode_AsWideChar*, PyUnicode_EncodeFSDefault and PyUnicode_AsUnicode*, since they're all concerned by this issue. arnaudc commented on Dec 18, 2011 I removed the hints "using wcslen on the result of PyUnicode_AsWideChar*", since the resulting wchar_t strings may not be null-terminated python-dev commented on Dec 18, 2011 python-dev commented on Dec 18, 2011 vstinner commented on Dec 18, 2011 embedded_nul-2.patch: a more complete patch check also null byte in functions calling PyUnicode_EncodeFSDefault(). serhiy-storchaka commented on Oct 17, 2012 I added some comments in Rietveld.
I see other instances of the use of non-checked PyUnicode_AsWideCharString() and PyUnicode_AsUnicode(). BreamoreBoy commented on Jun 29, 2014 @victor can you pick this up again please. serhiy-storchaka commented on Sep 5, 2014 Could you please answer my comments Victor? benhoyt commented on Feb 26, 2015 Note that this (or a very similar issue) also affects os.listdir() on Windows: os.listdir(bytes_path_with_nul) raises ValueError as expected, but os.listdir(unicode_path_with_nul) does not. Test case:
>>> import os
>>> os.mkdir('foo')
>>> os.listdir(b'foo\x00zzz')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: listdir: embedded null character in path
>>> os.listdir('foo\x00zzz')
[]However, this is not the case on Linux, as there both calls raise an appropriate ValueError.
This needs to be fixed in posixmodule.c's path_converter() function.
I'm in the middle of implementing PEP-471 (os.scandir), so don't want to create a proper patch right now, but the fix is to add these lines in posixmodule.c path_converter() after the if (length > 32767) {...} block:
if ((size_t)length != wcslen(wide)) {
FORMAT_EXCEPTION(PyExc_ValueError, "embedded null character in %s");
Py_DECREF(unicode);
return 0;
}We should also add test to test_os.py like the following:
def test_listdir_nul_in_path(self):
self.assertRaises(ValueError, os.listdir, 'y\x00z')
self.assertRaises(ValueError, os.listdir, b'y\x00z')serhiy-storchaka commented on Jun 17, 2017 Could you update your patch Victor? vstinner commented on Jun 19, 2017 Sorry, I lost track of this issue. Feel free to update and complete my patch :-) serhiy-storchaka commented on Jun 20, 2017 PR 2302 doesn't fix all issues with PyUnicode_AsWideCharString(). bpo-30708 should fix them. serhiy-storchaka commented on Jun 21, 2017 While working on this issue I found a way to inject environment variables for a subprocess on Windows. Reclassified this issue as a security issue. PR 2302 fixes this. May be there are other security vulnerabilities fixed by it. vstinner commented on Jun 28, 2017 Wow, it's nice to see activity on this issue that I opened 6 years ago :-)
Sorry Serhiy, I don't have the bandwidth right now to review your change :-( In lack of review, I suggest you to just push it. serhiy-storchaka commented on Jun 28, 2017 serhiy-storchaka commented on Jun 28, 2017 serhiy-storchaka commented on Jun 28, 2017 serhiy-storchaka commented on Jun 28, 2017 serhiy-storchaka commented on Jun 28, 2017 Backporting this to 2.7 requires too much work taking to account that PyArg_Parse and other argument parsing functions don't check for null characters in 2.7. The most serious security issue is fixed in bpo-30730, other cases unlikely can be used for attacks. vstinner commented on Jun 28, 2017 Thank you very much Serhiy of taking care of this bug!
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|