[issue6608] asctime causing python to crash
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: Here is a quote from the relevant CERT advisory (MSC33-C): This function is supposed to output a character string of 26 positions at most, including the terminating zero. If we count the length indicated by the format directives we arrive at 25. Taking into account the terminating zero, the array size of the string appears sufficient. However, this implementation assumes that the values of the struct tm data in timeptr are within normal ranges, and does nothing to enforce this. If any of the values print more characters than expected, the sprintf() function may overflow the result array. For instance, if tm_year has the value 12345, then 27 characters (including the terminating null character) are printed, resulting in a buffer overflow. The asctime() function primarily exists for compatibility with older implementations. Also, the asctime() function does not support localized date and time formats. The POSIX standard developers decided to mark the asctime() function obsolescent even though they are in C99 because of the possibility of buffer overflow. C99 also provides the strftime() function which can be used to avoid these problems. https://www.securecoding.cert.org/confluence/display/seccode/MSC33-C.+Do+not+pass+invalid+data+to+the+asctime%28%29+function (I am changing the stage back to needs patch because the current patch is vulnerable to buffer overflow.) I think it is best to leave the code as is and possibly add a warning in documentation that passing hand-crafted timetuple is unsafe on some systems and that locale aware strftime(%c, ..) is preferable to asctime. -- stage: patch review - needs patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: I have untabified James' patch, ran the tests on the result, but did not otherwise review it. There is a not-so-easy-to-find thread on python-dev discussing this issue under subject Python 2.6.5. Here is a relevant quote from Martin v. Löwis: That would require that Barry actually *can* judge the issue at hand. In the specific case, I would expect that Barry would defer the specifics of the Windows issue to Windows experts, and then listen to what they say. I'm personally split whether the proposed patch is correct (i.e. whether asctime really *can* be implemented in a cross-platform manner; any definite ruling on that would be welcome). In the past, we had rather taken approaches like disabling runtime assertions locally; not sure whether such approaches would work for asctime as well. In any case, I feel that the issue is not security-critical at all. People just don't pass out-of-range values to asctime, but instead typically pass the result of gmtime/localtime, which will not cause any problems. -- assignee: - belopolsky components: +Distutils -Extension Modules, Windows nosy: +belopolsky ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
Changes by Alexander Belopolsky belopol...@users.sourceforge.net: Added file: http://bugs.python.org/file17468/issue6608.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
Alexander Belopolsky belopol...@users.sourceforge.net added the comment: The patch as written causes buffer overflow for year = 10,000: len(time.asctime( (1, 1, 1, 0, 0, 0, 0, 1, -1))) 26 len(time.asctime( (10, 1, 1, 0, 0, 0, 0, 1, -1))) 27 while the buffer is only 26 characters: + static char result[26]; + + sprintf(result, %.3s %.3s%3d %.2d:%.2d:%.2d %d\n, This can be fixed in multiple ways: changing the year format to %.4d, using PyString_Format, or restricting the year to 4 decimal digits in check_bounds. A nit pick: you can save some static storage by making wday_name and mon_name and possibly increase performance of asctime 2d arrays instead of arrays of pointers to null-terminated strings. See http://www.opengroup.org/onlinepubs/009695399/functions/asctime.html . Just as Martin, I am split on whether the patch is correct. The fact that it is almost a copy of POSIX reference implementation gives some confidence, but that confidence is taken away by the reference implementation having a buffer overflow bug. I am also not sure that all systems produce the same end of line character. I would like to hear from Windows experts. -- components: +Extension Modules, Windows -Distutils stage: needs patch - patch review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
Antoine Pitrou pit...@free.fr added the comment: Retrograding to critical after popular python-dev demand. -- nosy: +pitrou priority: release blocker - critical ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
Changes by Sridhar Ratnakumar sridh...@activestate.com: -- nosy: +srid ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
Changes by Antoine Pitrou pit...@free.fr: -- nosy: +brett.cannon, lemburg priority: normal - release blocker ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
James Abbatiello abb...@gmail.com added the comment: Further investigation shows that MS asctime() doesn't like leap seconds and causes an assertion when passing (2008, 12, 31, 23, 59, 60, 2, 366, -1) - 'Wed Dec 31 23:59:60 2008'. Given that and since asctime() is such a simple function I think it makes more sense to roll our own. Attached is a rough patch which does this. It pulls the bounds checking used in strftime() out into its own function so it can be used in both places. Overriding ctime() is necessary because Windows decided to use %02d instead of %3d to print the day of the month. Without overriding ctime() the output of ctime(t) would be different from asctime(localtime(t)) and cause test_conversion() to fail. There is a USE_SYSTEM_ASCTIME switch to decide whether to use the system asctime() or the internal one. -- keywords: +patch Added file: http://bugs.python.org/file14628/asctime.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
James Abbatiello abb...@gmail.com added the comment: Since there's no good way to disable the assertion (see issue4804), checking the validity of the argument beforehand looks like an option. The checking that's currently being done in the strftime() implementation looks useful but it is not enough. The checking in the MS implementation of asctime() is very strict and validates the entire date, not just one field at a time. So there's no way to print out non-existant dates like (2009, 2, 31, 0, 0, 0, 0, 0, 0) - 'Mon Feb 31 00:00:00 2009'. I don't know if anybody is relying on that kind of behavior. If not then the function could be limited to accept only valid dates. Alternatively we could just not call down to asctime() but instead provide our own implementation using sprintf/strftime. -- nosy: +abbeyj ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
New submission from Amir Habibi a...@aryosys.com: I use: Python 2.6.2 (r262:71605, Apr 14 2009, 22:40:02) [MSC v.1500 32 bit (Intel)] on win32 import time time.asctime((2009, 1, 1, 24, 0, 0, 0, 0, 0)) the 24 is a wrong parameter but it should'n't crash the engine. This may be the side effect of a more serious problem though. An assertion may fix it. -- components: None messages: 91115 nosy: AmirHabibi severity: normal status: open title: asctime causing python to crash type: crash versions: Python 2.6 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue6608] asctime causing python to crash
Alexandre Vassalotti alexan...@peadrop.com added the comment: Confirmed. On Windows, the out-of-range value triggers a debugging assertion in the CRT library. -- components: +Extension Modules, Windows -None nosy: +alexandre.vassalotti priority: - normal stage: - needs patch versions: +Python 2.7, Python 3.1, Python 3.2 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue6608 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
