[issue23055] PyUnicode_FromFormatV crasher
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Serhiy Storchaka added the comment: Backported tests exposed off-by-one error in PyUnicode_FromFormatV. This error was fixed in 3.x in changeset ac768c8e13ac (issue7228). -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Roundup Robot added the comment: New changeset 245c9f372a34 by Serhiy Storchaka in branch '2.7': Issue #23055: Fixed read-past-the-end error in PyUnicode_FromFormatV. https://hg.python.org/cpython/rev/245c9f372a34 New changeset 9fe1d861f486 by Serhiy Storchaka in branch '3.2': Issue #23055: Fixed read-past-the-end error in PyUnicode_FromFormatV. https://hg.python.org/cpython/rev/9fe1d861f486 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Stefan Krah added the comment: I think in 2.7 there's a slight problem since e6b9e277fbf4: [1/1] test_unicode Debug memory block at address p=0x7f4ebba3fae0: API 'o' 100 bytes originally requested The 7 pad bytes at p-7 are FORBIDDENBYTE, as expected. The 8 pad bytes at tail=0x7f4ebba3fb44 are not all FORBIDDENBYTE (0xfb): at tail+0: 0x00 *** OUCH at tail+1: 0xfb at tail+2: 0xfb at tail+3: 0xfb at tail+4: 0xfb at tail+5: 0xfb at tail+6: 0xfb at tail+7: 0xfb The block was made by call #659785 to debug malloc/realloc. Data at p: 20 20 20 20 20 20 20 20 ... 20 20 20 20 20 31 32 33 Fatal Python error: bad trailing pad byte Aborted (core dumped) -- nosy: +skrah ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Serhiy Storchaka added the comment: Yes, I think following patch will help. -- Added file: http://bugs.python.org/file37929/issue23055_2.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Stefan Krah added the comment: issue23055_2.patch looks good. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Roundup Robot added the comment: New changeset e5d79e6deeb5 by Serhiy Storchaka in branch '2.7': Issue #23055: Fixed off-by-one error in PyUnicode_FromFormatV. https://hg.python.org/cpython/rev/e5d79e6deeb5 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Serhiy Storchaka added the comment: Thank you Stefan for pointing on tests failure. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Stefan Krah added the comment: I think I still get a problem in 2.7: [1/1] test_unicode ==23430== Invalid read of size 1 ==23430==at 0x484541: PyUnicodeUCS2_FromFormatV (unicodeobject.c:736) ==23430==by 0x485C75: PyUnicodeUCS2_FromFormat (unicodeobject.c:1083) 736 for (f = format; *f; f++) { (gdb) p format $1 = 0x71d45f4 % (gdb) p f $2 = 0x71d45f6 So format==%, first f++ happens at 738, second f++ happens implicitly at the end of the for loop. The *f condition in 736 is then an invalid read. Perhaps use while for the outer loop and a break? (It's just my personal preference, I sometimes get confused by incrementing at the end and also inside for loops.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Changes by Serhiy Storchaka storch...@gmail.com: -- status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Changes by Serhiy Storchaka storch...@gmail.com: -- status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Georg Brandl added the comment: It's fine to commit it to both branches. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Roundup Robot added the comment: New changeset e6b9e277fbf4 by Serhiy Storchaka in branch '2.7': Issue #23055: Fixed a buffer overflow in PyUnicode_FromFormatV. Analysis https://hg.python.org/cpython/rev/e6b9e277fbf4 New changeset f849f937f78c by Serhiy Storchaka in branch '3.2': Issue #23055: Fixed a buffer overflow in PyUnicode_FromFormatV. Analysis https://hg.python.org/cpython/rev/f849f937f78c New changeset 6caed177a028 by Serhiy Storchaka in branch '3.3': Issue #23055: Fixed a buffer overflow in PyUnicode_FromFormatV. Analysis https://hg.python.org/cpython/rev/6caed177a028 -- nosy: +python-dev ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Changes by Serhiy Storchaka storch...@gmail.com: -- resolution: - fixed stage: patch review - resolved status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Serhiy Storchaka added the comment: Georg, what is your word as release manager of 3.2/3.3? I would commit the patch in 2.7 if there are no objections. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Serhiy Storchaka added the comment: Here is updated patch for 2.7 (backported tests from 3.5), patches for 3.2 and 3.3. -- Added file: http://bugs.python.org/file37466/issue23055-2.7-2.patch Added file: http://bugs.python.org/file37467/issue23055-3.2.patch Added file: http://bugs.python.org/file37468/issue23055-3.3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___diff -r 9927781e457f Lib/test/test_unicode.py --- a/Lib/test/test_unicode.py Mon Dec 15 14:02:43 2014 +0200 +++ b/Lib/test/test_unicode.py Tue Dec 16 13:52:24 2014 +0200 @@ -1700,6 +1700,9 @@ class UnicodeTest( if sys.maxunicode 0x: check_format(u'\U0010', b'%c', c_int(0x10)) +else: +with self.assertRaises(OverflowError): +PyUnicode_FromFormat(b'%c', c_int(0x1)) with self.assertRaises(OverflowError): PyUnicode_FromFormat(b'%c', c_int(0x11)) # Issue #18183 @@ -1750,8 +1753,45 @@ class UnicodeTest( b'%zu', c_size_t(123)) # test long output +min_long = -(2 ** (8 * sizeof(c_long) - 1)) +max_long = -min_long - 1 +check_format(unicode(min_long), + b'%ld', c_long(min_long)) +check_format(unicode(max_long), + b'%ld', c_long(max_long)) +max_ulong = 2 ** (8 * sizeof(c_ulong)) - 1 +check_format(unicode(max_ulong), + b'%lu', c_ulong(max_ulong)) PyUnicode_FromFormat(b'%p', c_void_p(-1)) +# test padding (width and/or precision) +check_format(u'123'.rjust(10, u'0'), + b'%010i', c_int(123)) +check_format(u'123'.rjust(100), + b'%100i', c_int(123)) +check_format(u'123'.rjust(100, u'0'), + b'%.100i', c_int(123)) +check_format(u'123'.rjust(80, u'0').rjust(100), + b'%100.80i', c_int(123)) + +check_format(u'123'.rjust(10, u'0'), + b'%010u', c_uint(123)) +check_format(u'123'.rjust(100), + b'%100u', c_uint(123)) +check_format(u'123'.rjust(100, u'0'), + b'%.100u', c_uint(123)) +check_format(u'123'.rjust(80, u'0').rjust(100), + b'%100.80u', c_uint(123)) + +check_format(u'123'.rjust(10, u'0'), + b'%010x', c_int(0x123)) +check_format(u'123'.rjust(100), + b'%100x', c_int(0x123)) +check_format(u'123'.rjust(100, u'0'), + b'%.100x', c_int(0x123)) +check_format(u'123'.rjust(80, u'0').rjust(100), + b'%100.80x', c_int(0x123)) + # test %V check_format(u'repr=abc', b'repr=%V', u'abc', b'xyz') diff -r 9927781e457f Misc/NEWS --- a/Misc/NEWS Mon Dec 15 14:02:43 2014 +0200 +++ b/Misc/NEWS Tue Dec 16 13:52:24 2014 +0200 @@ -10,6 +10,9 @@ What's New in Python 2.7.10? Core and Builtins - +- Issue #23055: Fixed a buffer overflow in PyUnicode_FromFormatV. Analysis + and fix by Guido Vranken. + - Issue #23048: Fix jumping out of an infinite while loop in the pdb. Library diff -r 9927781e457f Objects/unicodeobject.c --- a/Objects/unicodeobject.c Mon Dec 15 14:02:43 2014 +0200 +++ b/Objects/unicodeobject.c Tue Dec 16 13:52:24 2014 +0200 @@ -735,15 +735,10 @@ PyUnicode_FromFormatV(const char *format * objects once during step 3 and put the result in an array) */ for (f = format; *f; f++) { if (*f == '%') { - if (*(f+1)=='%') - continue; - if (*(f+1)=='S' || *(f+1)=='R') - ++callcount; - while (isdigit((unsigned)*f)) - width = (width*10) + *f++ - '0'; - while (*++f *f != '%' !isalpha((unsigned)*f)) - ; - if (*f == 's') + f++; + while (*f *f != '%' !isalpha((unsigned)*f)) + f++; + if (*f == 's' || *f=='S' || *f=='R') ++callcount; } } @@ -760,12 +755,16 @@ PyUnicode_FromFormatV(const char *format /* step 3: figure out how large a buffer we need */ for (f = format; *f; f++) { if (*f == '%') { -const char* p = f; +const char* p = f++; width = 0; while (isdigit((unsigned)*f)) width = (width*10) + *f++ - '0'; -while (*++f *f != '%' !isalpha((unsigned)*f)) -; +precision = 0; +if (*f == '.') { +f++; +while (isdigit((unsigned)*f)) +precision = (precision*10) + *f++ - '0'; +} /* skip the 'l' or 'z' in {%ld, %zd, %lu, %zu} since * they
[issue23055] PyUnicode_FromFormatV crasher
New submission from Guido van Rossum: Fix as reported by Guido Vranken on secur...@python.org, with minimal test by me. Needs: - review - port to 3.2--3.5 Questions: - Does my test case cover all changed code? -- assignee: gvanrossum files: vranken.diff keywords: needs review, patch messages: 232673 nosy: gvanrossum priority: normal severity: normal status: open title: PyUnicode_FromFormatV crasher type: crash versions: Python 2.7, Python 3.4, Python 3.5 Added file: http://bugs.python.org/file37455/vranken.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Serhiy Storchaka added the comment: I have added a couple of comments. Here is a patch which fixes found bugs. 3.4+ is not affected by this bug. 3.2 looks same as 2.7 and is affected, 3.3 uses different code but at first glance looks affected too. Is it worth to fix this bug in 3.2 and 3.3? -- components: +Interpreter Core nosy: +georg.brandl, haypo, serhiy.storchaka stage: - patch review versions: -Python 3.4, Python 3.5 Added file: http://bugs.python.org/file37457/issue23055-2.7.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Guido van Rossum added the comment: Thank you for digging into this! I'd say go ahead and update 3.2 and 3.3 too -- these are in security-fix-only mode meaning that we only fix security issues and don't do actual releases. But we still do security bugfixes: for 3.2 until February 2016 (PEP 392), for 3.3 until September 2017 (PEP 398). I don't think that this warrants changing the 2.7 bugfix release schedule. -- versions: +Python 3.2, Python 3.3 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Guido Vranken added the comment: Serhiy Storchaka: good call on changing my 'n += (width + precision) 20 ? 20 : (width + precision);' into 'if (width precision) width = precision;', I didn't realize that sprintf's space requirement entails using the largest of the two instead of adding the two together. I noticed the apparently pointless width calculation in 'step 1' but decided not to touch it -- good that it's removed now though. I will start doing more debugging based on this new patch now to ensure that the bug is gone now. On a more design-related note, for the sake of readability and stability, I'd personally opt for implementing toned-down custom sprintf-like function that does exactly what it needs to do and nothing more, since a function like this one requires perfect alignment with the underlying sprintf() in terms of functionality, at the possible expense of stability and integrity issues like we see here. For instance, width and precision are currently overflowable, resulting in either a minus sign appearing in the resulant format string given to sprintf() (width and precision are signed integers), or completely overflowing it (ie. (uint64_t)18446744073709551617 == 1 ). Considering the latter example, how do we know sprintf uses the same logic? Guido -- nosy: +Guido ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Guido Vranken added the comment: I'd also like to add that, although I agree with Guido van Rossum that the likelihood of even triggering this bug in a general programming context is low, there are two buffer overflows at play here (one stack-based and one heap-based), and given an adversary's control over the format and vargs parameters, I'd there is a reasonable likelihood of exploiting it to execute arbitrary code, since the one controlling the parameters has some control as to which bytes end up where outside buffer boundaries. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue23055] PyUnicode_FromFormatV crasher
Guido van Rossum added the comment: I'd be much worried about attack scenarios if this function was part of the standard library. But it's not -- the stdlib's % operator uses completely different code. The most common use case is probably to generate error messages from extension modules -- and there the format is almost always a literal in the C code. (An adversary who can load a C extension doesn't need this exploit.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue23055 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com