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.

classification
Title: Use PyOS_snprintf for static buffers
Type: enhancement Stage: patch review
Components: Interpreter Core Versions: Python 3.2
process
Status: closed Resolution: wont fix
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, ajaksu2, amaury.forgeotdarc, calvin, loewis
Priority: normal Keywords: patch

Created on 2006-02-09 21:35 by calvin, last changed 2022-04-11 14:56 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
python-snprintf.diff calvin, 2006-02-09 21:35
python-pyos_snprintf.diff calvin, 2006-02-14 21:27
Messages (9)
msg49441 - (view) Author: Bastian Kleineidam (calvin) Date: 2006-02-09 21:35
Hi,

there are some uses of sprintf in the Python C code.
Unfortunately sprintf is known to cause buffer
overruns. To prevent this I have written a patch that
replaces sprintf with snprintf. To be on the safe side
(and since I don't know much of the C code internals) I
only changed static buffers, where the sizeof()
operator is known to work.

The patch is against SVN 42293, and tested on a i386
Debian Linux system.
msg49442 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2006-02-10 19:51
Logged In: YES 
user_id=21627

The patch has several problems:

1. It is unnecessary. The print calls actually *cannot*
cause buffer overruns, because in all cases, the buffers are
large enough.

2. The insertion of a trailing null-byte is unnecessary:
snprintf will already add that byte, even if it runs out of
space.

The first argument could be discarded, since using snprintf
can be considered as improving maintainability; point 2
actually decreases maintainability.
msg49443 - (view) Author: Bastian Kleineidam (calvin) Date: 2006-02-13 19:13
Logged In: YES 
user_id=9205

The snprintf had implementations which did not
null-terminate the buffer if it was too small. I detected
that Python itself has a wrapper function defined for that
reason: PyOS_snprintf.

I will attach a patch in the next days that makes use of
PyOS_snprintf, which should remove the trailing null-byte lines.

I noted that none of the calls of both sprintf and
PyOS_snprintf actually check the return code. But I found
out that Guido does not care, as said in this message:
http://permalink.gmane.org/gmane.comp.python.devel/33591
msg49444 - (view) Author: Bastian Kleineidam (calvin) Date: 2006-02-14 21:27
Logged In: YES 
user_id=9205

I added an updated patch that uses PyOS_snprintf, and only
in places where the Python headers are included.
msg87711 - (view) Author: Daniel Diniz (ajaksu2) * (Python triager) Date: 2009-05-13 21:17
Does anyone want to adopt this one? :)
msg110445 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-16 14:16
I'm torn by this one.  Half of me says it's a good thing trying to be fail safe.  The other half says it's work for something that hasn't yet happened.  Any other views?
msg110458 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-07-16 15:50
Most usages of sprintf here cannot cause buffer overruns: the output is bounded in size (%d, %8.8x, %.200s), and the buffer is large enough.

Moreover, some of them were already replaced by functions of the _FromFormat() family, which can handle unicode for example.

IMO the change is not worth it.
msg110459 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-16 15:55
I'll close unless someone puts in a strong bid to keep this open.
msg110819 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2010-07-19 22:27
Closing as noone has responded.
History
Date User Action Args
2022-04-11 14:56:15adminsetgithub: 42880
2010-07-19 22:27:08BreamoreBoysetstatus: pending -> closed
resolution: wont fix
messages: + msg110819
2010-07-16 15:55:00BreamoreBoysetstatus: open -> pending

messages: + msg110459
2010-07-16 15:50:38amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg110458
2010-07-16 14:16:04BreamoreBoysetversions: - Python 2.6, Python 3.1, Python 2.7
nosy: + BreamoreBoy

messages: + msg110445

type: enhancement
2009-05-13 21:17:04ajaksu2setversions: + Python 3.1, Python 2.7, Python 3.2, - Python 2.5
nosy: + ajaksu2

messages: + msg87711

stage: patch review
2008-01-05 19:53:04christian.heimessetversions: + Python 2.6, Python 2.5
2006-02-09 21:35:12calvincreate