This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author alexandre.vassalotti
Recipients alexandre.vassalotti, belopolsky
Date 2008-03-10.02:18:56
SpamBayes Score 0.011905444
Marked as misclassified No
Message-id <1205115539.28.0.731341554426.issue1950@psf.upfronthosting.co.za>
In-reply-to
Content
Thanks for the review!

> Your description of the patch is a bit misleading.  As far as I can
> tell only the first chunk (Python/import.c changes) addresses a
> potential buffer overflow.

Yes, you are right. It seems only the bug in import.c could easily be
exploited.

> 1. It will really help if you produce a test case that crashes the
> interpretor.  I am sure that will get noticed.

   % cat pkg/__init__.py
   __package__ = "\U000c9c9c9" * 900
   from . import f
   % ./python
   Python 3.0a3+ (py3k:61164, Mar  1 2008, 19:55:42)
   >>> import pkg
   *** stack smashing detected ***: ./python terminated
   [1]    9503 abort (core dumped)  ./python

> 2. If any of buffer overflows apply to the current production
> versions (2.4 or 2.5) or even the alpha release (2.6a1), it would
> make sense to backport it to the trunk.

I don't think the trunk is affected in any way by the issues
mentioned here.

> The existing __doc__ processing code is correct.  Proposed code may be
> marginally faster, but will allow docstrings with embedded null
> characters, which may or may not be desirable (and may break other code
> that uses tp_doc)

Good call! I will check out if null-characters may pose a problem for
tp_doc and update the patch consequently.

> I don't think a repr of a python object can contain embedded null
> characters, but even if that were the case, the patched code would not
> support it because the resulting buffer is returned with
> PyUnicode_FromString(buf).

Oh, that is true. I will remove that part from the patch, then.

> Modules/datetimemodule.c
>
> Existing code compensates for an error in initial estimate of totalnew
> when it checks for overflow, but the proposed change will make code more
> efficient.

Right again.

> Modules/zipimport.c
[...]
> Modules/timemodule.c
[...]
> Modules/parsermodule.c
[...]

I need to check again the code for these three modules, before
commenting.

I will clean up the patch with your recommendation and post it
again. Thanks for taking the time to review my patch. It's greatly
appreciated.
History
Date User Action Args
2008-03-10 02:18:59alexandre.vassalottisetspambayes_score: 0.0119054 -> 0.011905444
recipients: + alexandre.vassalotti, belopolsky
2008-03-10 02:18:59alexandre.vassalottisetspambayes_score: 0.0119054 -> 0.0119054
messageid: <1205115539.28.0.731341554426.issue1950@psf.upfronthosting.co.za>
2008-03-10 02:18:58alexandre.vassalottilinkissue1950 messages
2008-03-10 02:18:56alexandre.vassalotticreate