Replace useless %.100s by %s in PyErr_Format()
Hi, I plan to replace all %.100s (or any other size, %\.[0-9]+s regex) by %s in the whole source code, in all calls to PyErr_Format(). And I would like your opinion. When Guido added the function PyErr_Format(), 13 years ago, the function was implemented using a buffer of 500 bytes (allocated on the stack). The developer was responsible to limit the argument fit into a total of 500 bytes. But 3 years later (2000), PyErr_Format() was patched to use a dynamic buffer (allocated on the heap). But since this change, PyErr_Format() doesn't support %.100s anymore (the 100 bytes limitation is just ignored), and it becomes useless and so no, it's no more (since 10 years) a "protection" against segmentation fault. But I would like to know if I have to do in all branches (3.1-3.3, or worse: 2.5-3.3), or just in 3.3? Because it may make merge harder (like any change only done in default). I would like to replace %.100s because there are no more reason to truncate strings to an arbitrary length. => http://bugs.python.org/issue10833 --- ... at the same time, Ray Allen wrote a patch to implement %.100s in PyUnicode_FromFormatV() (so PyErr_Format() will support it too). I would like to replace %.100s in PyErr_Format(), and then commit its patch. http://bugs.python.org/issue7330 Victor
Victor Stinner wrote:
Hi,
I plan to replace all %.100s (or any other size, %\.[0-9]+s regex) by %s in the whole source code, in all calls to PyErr_Format(). And I would like your opinion.
When Guido added the function PyErr_Format(), 13 years ago, the function was implemented using a buffer of 500 bytes (allocated on the stack). The developer was responsible to limit the argument fit into a total of 500 bytes. But 3 years later (2000), PyErr_Format() was patched to use a dynamic buffer (allocated on the heap). But since this change, PyErr_Format() doesn't support %.100s anymore (the 100 bytes limitation is just ignored), and it becomes useless and so no, it's no more (since 10 years) a "protection" against segmentation fault.
But I would like to know if I have to do in all branches (3.1-3.3, or worse: 2.5-3.3), or just in 3.3? Because it may make merge harder (like any change only done in default).
I would like to replace %.100s because there are no more reason to truncate strings to an arbitrary length.
=> http://bugs.python.org/issue10833
---
... at the same time, Ray Allen wrote a patch to implement %.100s in PyUnicode_FromFormatV() (so PyErr_Format() will support it too). I would like to replace %.100s in PyErr_Format(), and then commit its patch.
I think it's better to add the #7330 fix and leave the length limitations in place. Note that the length limitation did not only protect against segfaults at the time when PyErr_Format() was using a fixed size buffer and later on against miscalculations in creating the variable sized buffer, it also protects against making the error message text too long to be of any use or cause problems further down the line in error processing. BTW: Why do you think that %.100s is not supported in PyErr_Format() in Python 2.x ? PyString_FromFormatV() does support this. The change to use Unicode error strings introduced the problem, since PyUnicode_FromFormatV() for some reason ignores the precision (which is shouldn't). That said, it's a good idea to add the #7330 fix to at least Python 2.7 as well, since ignoring the precision is definitely a bug. It may even be security relevant, since it could be used for DOS attacks on servers (e.g. causing them to write huge strings to log files instead of just a few hundreds bytes per message), so may even need to go into Python 2.6. Thanks, -- Marc-Andre Lemburg eGenix.com Professional Python Services directly from the Source (#1, Mar 24 2011)
Python/Zope Consulting and Support ... http://www.egenix.com/ mxODBC.Zope.Database.Adapter ... http://zope.egenix.com/ mxODBC, mxDateTime, mxTextTools ... http://python.egenix.com/
::: Try our new mxODBC.Connect Python Database Interface for free ! :::: eGenix.com Software, Skills and Services GmbH Pastor-Loeh-Str.48 D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg Registered at Amtsgericht Duesseldorf: HRB 46611 http://www.egenix.com/company/contact/
Le jeudi 24 mars 2011 à 13:22 +0100, M.-A. Lemburg a écrit :
BTW: Why do you think that %.100s is not supported in PyErr_Format() in Python 2.x ? PyString_FromFormatV() does support this. The change to use Unicode error strings introduced the problem, since PyUnicode_FromFormatV() for some reason ignores the precision (which is shouldn't).
Oh... You are right, it is a regression in Python 3. We started to write unit tests for PyBytes_FromFormat() and PyUnicode_FromFormat(), I hope that they will improve the situation.
That said, it's a good idea to add the #7330 fix to at least Python 2.7 as well, since ignoring the precision is definitely a bug. It may even be security relevant, since it could be used for DOS attacks on servers (e.g. causing them to write huge strings to log files instead of just a few hundreds bytes per message), so may even need to go into Python 2.6.
Python 2 is not affected because PyErr_Format() uses PyString_FromFormatV() which supports precision for %s format (e.g. %.100s truncate the string to 100 bytes). Do you think that Python 3.1-3.3 should be fixed? Let's begin by closing #10833 as invalid. Victor
Victor Stinner wrote:
Le jeudi 24 mars 2011 à 13:22 +0100, M.-A. Lemburg a écrit :
BTW: Why do you think that %.100s is not supported in PyErr_Format() in Python 2.x ? PyString_FromFormatV() does support this. The change to use Unicode error strings introduced the problem, since PyUnicode_FromFormatV() for some reason ignores the precision (which is shouldn't).
Oh... You are right, it is a regression in Python 3. We started to write unit tests for PyBytes_FromFormat() and PyUnicode_FromFormat(), I hope that they will improve the situation.
That said, it's a good idea to add the #7330 fix to at least Python 2.7 as well, since ignoring the precision is definitely a bug. It may even be security relevant, since it could be used for DOS attacks on servers (e.g. causing them to write huge strings to log files instead of just a few hundreds bytes per message), so may even need to go into Python 2.6.
Python 2 is not affected because PyErr_Format() uses PyString_FromFormatV() which supports precision for %s format (e.g. %.100s truncate the string to 100 bytes).
Right, but the PyUnicode_FromFormatV() which ignores the precision is still present in Python 2.6 and 2.7, even though it is not used by PyErr_Format().
Do you think that Python 3.1-3.3 should be fixed?
Yes, indeed. The above mentioned security threat is real. The CPython code only has a few cases where this could be use for a DOS (e.g. in the pickle module or the AST code), but since this function is used in 3rd party extensions, those are affected indirectly as well. Thanks, -- Marc-Andre Lemburg eGenix.com Professional Python Services directly from the Source (#1, Mar 30 2011)
Python/Zope Consulting and Support ... http://www.egenix.com/ mxODBC.Zope.Database.Adapter ... http://zope.egenix.com/ mxODBC, mxDateTime, mxTextTools ... http://python.egenix.com/
::: Try our new mxODBC.Connect Python Database Interface for free ! :::: eGenix.com Software, Skills and Services GmbH Pastor-Loeh-Str.48 D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg Registered at Amtsgericht Duesseldorf: HRB 46611 http://www.egenix.com/company/contact/
I would like to replace %.100s because there are no more reason to truncate strings to an arbitrary length.
I agree with MAL. It protects against cases with ridiculously long parameters - say, you have a string with 1GB. You *want* to truncate bogus text. Regards, Martin
participants (3)
-
"Martin v. Löwis" -
M.-A. Lemburg -
Victor Stinner