classification
Title: bytearray().count()
Type: crash Stage:
Components: Interpreter Core Versions: Python 3.0, Python 2.6, Python 2.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: amaury.forgeotdarc, benjamin.peterson, haypo
Priority: release blocker Keywords: needs review, patch

Created on 2008-09-25 21:39 by haypo, last changed 2008-09-26 22:49 by amaury.forgeotdarc. This issue is now closed.

Files
File name Uploaded Description Edit
string_count.patch amaury.forgeotdarc, 2008-09-26 00:02
Messages (8)
msg73817 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2008-09-25 21:39
bytes_count() doesn't check start maximum value: _adjust_indices()
should check that start is smaller than len (smaller or egal? len or
len-1?). Example:

>>> b = bytearray(3)
>>> b.count("x", 1491491034, 0)

start=1491491034 should be replaced by 3 (or 2 or 4? I don't know
bytearray enough).
msg73820 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-25 22:43
Is there a problem, a potential crash?
looking at the code, I could not find any: in fastsearch, the "if (w < 
0)" will quickly exit the function, without reading the contents of the 
array.
msg73821 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2008-09-25 22:58
Here is a trace of Valgrind:

 >>> b=bytearray(2)
 >>> b.count("xxxx", 3493403, 0)
0
 >>> b.count("xxxx", 23131230123012010231023, 0)
==13650== Invalid read of size 1
==13650==    at 0x812718A: fastsearch (fastsearch.h:67)
==13650==    by 0x81272CB: stringlib_count (count.h:22)
==13650==    by 0x8128611: bytes_count (bytearrayobject.c:1198)
==13650==    by 0x81376B6: PyCFunction_Call (methodobject.c:81)
==13650==    by 0x80D913D: call_function (ceval.c:3679)
==13650==    by 0x80D603A: PyEval_EvalFrameEx (ceval.c:2370)
==13650==    by 0x80D7891: PyEval_EvalCodeEx (ceval.c:2942)
==13650==    by 0x80D0A8F: PyEval_EvalCode (ceval.c:515)
==13650==    by 0x80FB4FE: run_mod (pythonrun.c:1330)
==13650==    by 0x80FA166: PyRun_InteractiveOneFlags (pythonrun.c:836)
==13650==    by 0x80F9EEA: PyRun_InteractiveLoopFlags (pythonrun.c:756)
==13650==    by 0x80F9DAC: PyRun_AnyFileExFlags (pythonrun.c:725)
==13650==  Address 0x8444553a is not stack'd, malloc'd or (recently) free'd
==13650==
==13650== Process terminating with default action of signal 11 (SIGSEGV)

Trace of gdb:
0x0812718a in fastsearch (s=0x89b9fcaf <Address 0x89b9fcaf out of 
bounds>, n=-2147483647,
    p=0xb7b54274 "xxxx", m=4, mode=0) at Objects/stringlib/fastsearch.h:67
67              if (s[i+m-1] == p[m-1]) {
(gdb) where
#0  0x0812718a in fastsearch (s=0x89b9fcaf <Address 0x89b9fcaf out of 
bounds>, n=-2147483647,
    p=0xb7b54274 "xxxx", m=4, mode=0) at Objects/stringlib/fastsearch.h:67
#1  0x081272cc in stringlib_count (str=0x89b9fcaf <Address 0x89b9fcaf 
out of bounds>,
    str_len=-2147483647, sub=0xb7b54274 "xxxx", sub_len=4) at 
Objects/stringlib/count.h:22
#2  0x08128612 in bytes_count (self=0xb7d025f0, args=0xb7b5075c)
    at Objects/bytearrayobject.c:1198
(gdb) print w
$1 = 2147483645
(gdb) print m
$2 = 4

Oh, look at stringlib_count (..., str_len=-2147483647, ..., sub_len=4): 
str_len is negative.
msg73828 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-26 00:02
Ah, big numbers that overflow: even if 'start' is (silently) capped to  
sys.maxint-1,    len(b)-start-len("xx")   will wrap and yield a positive 
number...

The find/rfind/index/rindex methods have the same problem.

Attached a patch and unit tests, needs review.
msg73829 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-26 00:07
All versions are involved.

Even 2.4 has surprises:
>>> "aa".count("", sys.maxint, 0)
-2147483646
msg73898 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-09-26 21:57
I think the patch is good.
msg73902 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2008-09-26 22:45
Fixed in Python trunk, rev66631, by amaury.forgeotdarc.
msg73903 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2008-09-26 22:49
Committed in trunk: r66631, in release25-maint: r66632 and py3k: r66633.
History
Date User Action Args
2008-09-26 22:49:58amaury.forgeotdarcsetstatus: open -> closed
resolution: fixed
messages: + msg73903
2008-09-26 22:45:07hayposetmessages: + msg73902
2008-09-26 21:57:32benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg73898
2008-09-26 00:07:02amaury.forgeotdarcsetmessages: + msg73829
versions: + Python 2.5, Python 3.0
2008-09-26 00:02:28amaury.forgeotdarcsetpriority: release blocker
keywords: + needs review, patch
messages: + msg73828
files: + string_count.patch
2008-09-25 22:58:30hayposetmessages: + msg73821
2008-09-25 22:43:17amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg73820
2008-09-25 21:39:32haypocreate