[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Eryk Sun]

Change by Eryk Sun :


--
components: +ctypes

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Eryk Sun]

Eryk Sun  added the comment:

> The Documentation component is for issues that only change the docs

That's not clear in the triaging guide for the multi-select component field. 
(However, it is clearly stated as such for the GitHub PR label 
"type-documentation".) If that's really the case, then I'll manually nosy the 
docs team in cases such as this. When wording is disputed and needs to be 
clarified, I want an expert at documentation to propose or review the change.

> Adding 'Documentation' amounts to rejecting this patch or anything 
> else that changes the code.

That was not my intent. I accepted Tom's position that "string" means a C 
string, which must be null-terminated. So I added the "Lib" tag and left it as 
a "behavior" issue instead of changing it to "enhancement". 

I'm concerned that the old c_buffer() function is defined to call 
create_string_buffer(), and it's not officially deprecated in the docs or the 
source code. (There's a commented-out deprecation warning.) A related concern 
is that the documentation says that the length of a byte-string initializer 
should not be used if the size is specified, which allows creating a 
character-array that's not null-terminated. If it raises a ValueError in this 
case, the wording should be clear that the value of `size` must be large enough 
to set the initial value as a null-terminated string. I also would want 
c_buffer() to get a separate implementation in this case. 

If accepted, create_unicode_buffer(init, size) should also be changed to 
require that init is set as a null-terminated string.

--
components: +Library (Lib)
priority: low -> normal
stage: needs patch -> patch review
versions: +Python 3.10, Python 3.8, Python 3.9 -Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Berker Peksag]

Changes by Berker Peksag :


--
components:  -Devguide

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Terry J. Reedy]

Terry J. Reedy added the comment:

(Tracker notes:

I added as nosy the people listed as active 'experts' for ctypes on 
https://docs.python.org/devguide/experts.html#experts.  This was easily done by 
going to the end of the nosy list, typing a comma ',', typing 'ctypes', and 
then clicking the box that appeared.  This can be done for any module and the 
other topics listed on the page.

The Documentation component is for issues that only change the docs, and not 
the code.  That is why Documentation issues are auto-assigned to docs@python.  
Adding 'Documentation' amounts to rejecting this patch or anything else that 
changes the code.

asyncio, ctypes, IDLE (idlelib), IO, and (T)tkinter are all parts of the stdlib 
and AFAIK, issues marked for them do not have to also be marked 'Library'.)
---

I looked at ctypes.py with hg annotate.  Create_string_buffer is part of Thomas 
Heller's original 2006-03-08 patch that moved ctypes from an external source 
into the stdlib.  The only changes are in the isinstance class checks and the 
raise statement; the conditional bodies, including the one in question, are 
unchanged.

Tom, we disagree on our reading of the current docs.  The default number of 
NULL bytes added is 1.  Is the second argument required to be large enough to 
keep the number positive?  You think yes, I think no, though I agree with Eryk 
that the second quoted sentence could and should be clearer.  I will not assume 
that T. Heller meant 'yes' when he wrote 'no' in the code.  What do the listed 
experts think?

If the doc matches the code, there is no implementation bug and this is not a 
behavior issue. It is still possible to request a design change as an 
enhancement.  I think this would require agreement of at least two core 
developers.  A deprecation notice would normally be needed.  A third 
possibility is to decide that this is a security issue severe enough to 
possibly break code in 3.6 and possibly sooner.  I think this would require 
pydev discussion.

One problem with changing ctypes is that it is not used in the stdlib, so we 
have no local examples to draw on.  In this case, the question would be how 
often is 'size' used to suppress the default NULL byte and how legitimate are 
such uses.

--
components: +Devguide -Documentation, Library (Lib), ctypes
nosy: +amaury.forgeotdarc, belopolsky, ezio.melotti, meador.inge, terry.reedy, 
willingc

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Krista Paasonen]

Krista Paasonen added the comment:

Patch containing checking for buffer size, so that NULL value is the last byte 
as C standard specifies. Raises ValueError exception if initial value does not 
fit into to the buffer with NULL char.

This should decrease the possibility of creating security issues.

--
keywords: +patch
nosy: +krista
Added file: http://bugs.python.org/file41558/create_string_buffer.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Eryk Sun]

Eryk Sun added the comment:

I didn't want to change the function in lieu of breaking someone's code. If 
this change is accepted, then it at least needs a documentation note to 
indicate the new behavior.

--
versions: +Python 3.6 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Eryk Sun]

Changes by Eryk Sun :


--
components: +Documentation, Library (Lib)

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Tom Pohl]

New submission from Tom Pohl:

From the ctypes.create_string_buffer docs:
If a bytes object is specified as first argument, the buffer is made one 
item larger than its length so that the last element in the array is a NUL 
termination character. An integer can be passed as second argument which allows 
to specify the size of the array if the length of the bytes should not be 
used.

Based on this documentation I would expect a NUL-terminated byte array in any 
case. However, when I do this

 for size in range(5, 2, -1): print(size, 
 ctypes.create_string_buffer(b'123', size).raw)
5 b'123\x00\x00'
4 b'123\x00'
3 b'123'

I get b'123' for size=3 without a NUL. My expectation would be the same 
exception as I get for create_string_buffer(b'123', 2).

--
components: ctypes
messages: 248183
nosy: tom.pohl
priority: normal
severity: normal
status: open
title: ctypes.create_string_buffer does not add NUL if len(init) == size
type: behavior
versions: Python 3.4

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24823
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[eryksun]

eryksun added the comment:

Not every buffer is null-terminated. That's just the assumption used if the 
size isn't specified. The documentation can possibly be reworded to make this 
clearer, but the function itself shouldn't be changed.

--
assignee:  - docs@python
components: +Documentation
keywords: +easy
nosy: +docs@python, eryksun
priority: normal - low
stage:  - needs patch
versions: +Python 2.7, Python 3.5, Python 3.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24823
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Tom Pohl]

Tom Pohl added the comment:

I agree: not every buffer is null-terminated.

But the function name suggests that it creates a _string_ buffer which will 
most likely be used as an input to a C function. There, it can easily trigger a 
buffer overflow without a null termination which can be considered a severe 
security risk.

--
components:  -Documentation
versions:  -Python 2.7, Python 3.5, Python 3.6

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24823
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24823] ctypes.create_string_buffer does not add NUL if len(init) == size

[Tom Pohl]

Tom Pohl added the comment:

If one needs to set a general buffer (i.e. not a null-terminated string buffer) 
one could always use:

 string = (ctypes.c_char*4)()
 string.raw = b'abcd'

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24823
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com