[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Petr Splichal p...@gmail.com added the comment: Justin, is there any reproducer available for this issue? Thanks! -- nosy: +psss ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue2588 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Changes by Gregory P. Smith [EMAIL PROTECTED]: -- assignee: - gregory.p.smith keywords: +patch nosy: +gregory.p.smith priority: - normal __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Justin Ferguson [EMAIL PROTECTED] added the comment: Actually, I'm not sure things are any better today- even the same operating system but different versions have inconsistencies, for instance on some versions of RHEL the vsnprintf() can fail during unicode conversion. MSVCRT still returns -1 on truncation, et cetera. That said, theres plenty of other implementations that manage this without the potential of underflowing a buffer __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Alexander Belopolsky [EMAIL PROTECTED] added the comment: On Wed, Apr 9, 2008 at 1:16 PM, Justin Ferguson [EMAIL PROTECTED] wrote: .. That said, theres plenty of other implementations that manage this without the potential of underflowing a buffer Do you have in mind something like the following? === --- Python/mysnprintf.c (revision 62211) +++ Python/mysnprintf.c (working copy) @@ -88,6 +88,7 @@ PyMem_FREE(buffer); Done: #endif - str[size-1] = '\0'; + if (size 0) + str[size-1] = '\0'; return len; } I would be +0 on such change. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Justin Ferguson [EMAIL PROTECTED] added the comment: Yep, that works for me. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
New submission from Justin Ferguson [EMAIL PROTECTED]:
The PyOS_vsnprintf() contains the caveat that the length parameter
cannot be zero, however this is only enforced via assert() which is
compiled out. As a result if the length parameter is zero then the
function will underflow and write a null byte to invalid memory.
53 int
54 PyOS_vsnprintf(char *str, size_t size, const char *format, va_list va)
55 {
56 int len; /* # bytes written, excluding \0 */
57 #ifndef HAVE_SNPRINTF
58 char *buffer;
59 #endif
60 assert(str != NULL);
61 assert(size 0);
62 assert(format != NULL);
[...]
65 len = vsnprintf(str, size, format, va);
[...]
91 str[size-1] = '\0';
92 return len;
93 }
--
components: Distutils
messages: 65174
nosy: jnferguson
severity: normal
status: open
title: PyOS_vsnprintf() underflow leads to memory corruption
type: security
versions: Python 2.5
__
Tracker [EMAIL PROTECTED]
http://bugs.python.org/issue2588
__
___
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Changes by Justin Ferguson [EMAIL PROTECTED]: -- components: +Interpreter Core -Distutils __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Amaury Forgeot d'Arc [EMAIL PROTECTED] added the comment: I think that programming errors against the python API are best checked with asserts: I develop in development mode (with asserts enabled), then I want my released program to run at full speed. Other thoughts? -- nosy: +amaury.forgeotdarc __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Justin Ferguson [EMAIL PROTECTED] added the comment: I can generally agree with that, and I admit I haven't verified all of the code paths here- theres several hundred of them leading into this function, are you positive all of them are safe? (seems like it would be easier to just move the check into an if than sitting down and verifying that all XXX hundred code paths are safe). In the other bug, I have verified code paths into it, for instance test the misallocation poc in 2586 as an example __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Alexander Belopolsky [EMAIL PROTECTED] added the comment: As long as snprintf is used with a fixed size buffer using an idiom snprintf(buffer, sizeof(buffer), ..) there is no issue because sizeof(buffer) cannot be zero. AFAICT, this is how python uses PyOS_vsnprintf wrapper. On the other hand, may this is a good opportunity to revisit the decision to make PyOS_vsnprintf semantics different from C99 vsnprintf. C99 defines snprintf semantics as follows: int snprintf(char *restrict s, size_t n, const char *restrict format, ...); The snprintf() function shall be equivalent to sprintf(), with the addition of the n argument which states the size of the buffer referred to by s. If n is zero, nothing shall be written and s may be a null pointer. Otherwise, output bytes beyond the n-1st shall be discarded instead of being written to the array, and a null byte is written at the end of the bytes actually written into the array. http://www.opengroup.org/onlinepubs/95399/functions/printf.html -- nosy: +belopolsky __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Justin Ferguson [EMAIL PROTECTED] added the comment: I do agree with your point about snprintf(..., sizeof(x), ...)-- my single biggest point (and maybe i'm just not seeing it), is that there appears to be no good reason for having this caveat and in turn its essentially just code waiting to break; with as commonly used of a function as it is, it's really a matter of when and not so much if. While no one seems to ever use it this way, don't forget that a good alternative to asprintf() is calling sprintf() with a length of zero to get the length (in compliant implementations), allocating the memory and then calling it again. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue2588] PyOS_vsnprintf() underflow leads to memory corruption
Alexander Belopolsky [EMAIL PROTECTED] added the comment: On Tue, Apr 8, 2008 at 9:21 PM, Justin Ferguson [EMAIL PROTECTED] wrote: .. While no one seems to ever use it this way, don't forget that a good alternative to asprintf() is calling sprintf() with a length of zero to get the length (in compliant implementations), allocating the memory and then calling it again. Remember that PyOS_vsnprintf was introduced back in 2001 when (according to the comments in the file) not all platforms provided c99 compliant implementations. If you can verify that the situation has changes for the supported platforms, I think you will have a good case for making the wrapper c99 compliant. __ Tracker [EMAIL PROTECTED] http://bugs.python.org/issue2588 __ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
