[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Changes by Serhiy Storchaka :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

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

[issue22643] Integer overflow in case_operation

[Roundup Robot]

Roundup Robot added the comment:

New changeset 5fae49ef94fd by Serhiy Storchaka in branch '3.4':
Issue #22643: Skip test_case_operation_overflow on computers with low memory.
https://hg.python.org/cpython/rev/5fae49ef94fd

New changeset 6b00bee218ff by Serhiy Storchaka in branch '3.5':
Issue #22643: Skip test_case_operation_overflow on computers with low memory.
https://hg.python.org/cpython/rev/6b00bee218ff

New changeset b1c5949a3af4 by Serhiy Storchaka in branch 'default':
Issue #22643: Skip test_case_operation_overflow on computers with low memory.
https://hg.python.org/cpython/rev/b1c5949a3af4

--

___
Python tracker 

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

[issue22643] Integer overflow in case_operation

[STINNER Victor]

STINNER Victor added the comment:

 self.assertRaises(OverflowError, (�*(2**32//12 + 1)).upper)
 MemoryError

Hum, even with the PEP 393, this string is still large: 682 MB.

$ python3
Python 3.4.1 (default, Nov  3 2014, 14:38:10) 
 sys.getsizeof(�*(2**32//12 + 1)) / 1024.**2
682.6667385101318

I guess that Serhiy suggests to put a @bigmemtest decorator on 
test_case_operation_overflow().

The test expects that large_string.upper() raises immediatly an OverflowError, 
so I understand that a new string is not created.

Hum, it's strange to have an OverflowError here. Integer overflow on memory 
allocation usually raises MemoryError, not OverflowError. It looks like 
unicodeobject.c is not consistent here.

--
resolution: fixed - 
status: closed - open

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

http://buildbot.python.org/all/builders/PPC64%20AIX%203.x/builds/3466/steps/test/logs/stdio
==
ERROR: test_case_operation_overflow (test.test_unicode.UnicodeTest)
--
Traceback (most recent call last):
  File 
/home/shager/cpython-buildarea/3.x.edelsohn-aix-ppc64/build/Lib/test/test_unicode.py,
 line 852, in test_case_operation_overflow
self.assertRaises(OverflowError, (�*(2**32//12 + 1)).upper)
MemoryError

--

--

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

It's strange to have � here. Should be ü.

--

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

[issue22643] Integer overflow in case_operation

[STINNER Victor]

STINNER Victor added the comment:

 It's strange to have � here. Should be ü.

Oh right, I saw � in the bug tracker and in the buildbot output. But
it's a web browser issue, if you download the raw buildbout output,
you will see ü. So the string takes less memory (1 byte per
character, not 2).

--

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

bigimem tests are ran with dummy size = 5147 if dry_run = True (default) and 
skipped if dry_run = False. Almost always you need dry_run = False.

The -M option doesn't work with less than 1 GiB.

In this case likely the buildbot had enough memory at the start, but then some 
memory leaked in tests (perhaps mainly in tracebacks of failed tests).

--

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Changes by Serhiy Storchaka storch...@gmail.com:


--
keywords: +patch
stage: resolved - patch review
versions:  -Python 3.3
Added file: 
http://bugs.python.org/file38818/test_case_operation_overflow_memory_error.patch

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Perhaps adding bigmemtest is not worth if memory requirements are less than 1 
GiB. But I would separate the creating of long tested string and the calling of 
case converting method, so we will sure what operation is failed. Proposed 
patch skips the test if can't create needed string and clean locals, so failing 
test_case_operation_overflow no longer causes multiple MemoryErrors in 
following tests.

--

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

[issue22643] Integer overflow in case_operation

[STINNER Victor]

STINNER Victor added the comment:

 Perhaps adding bigmemtest is not worth if memory requirements are less than 1 
 GiB.

What is the current limit if you don't pass -M option to regrtest? In 
@bigimemtest, I see maxsize = 5147. Is it a number of bytes? 5 kB of memory?

IMO it's interested to use @bigmemtest if a test uses 100 MB of memory or more, 
but regrtest should use tests using less than 500 MB by default (or maybe less 
than 1 GB by default).

--

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

[issue22643] Integer overflow in case_operation

[Arfrever Frehtes Taifersar Arahesis]

Arfrever Frehtes Taifersar Arahesis added the comment:

Summary of commits:
  3.3:
449b1f427cc7
6a91e616485a
f963cc1f96cf
cda907a02a80
b4a6be12a4bc
  3.4:
570e70252d5d
a6d0b1275d4b
2a25076c50ad
33290d0dd946
2f90ea9e60ef
  3.5:
c2980ec10a4c
d0f8f242531b
8195d48a5c43
ffabb674140c
030fda7b1de8

--
nosy: +Arfrever
versions: +Python 3.3, Python 3.5

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

[issue22643] Integer overflow in case_operation

[paul]

New submission from paul:

Crashes python 3.4.1. 

# Objects\unicodeobject.c
# 
# static PyObject *
# case_operation(PyObject *self,
#Py_ssize_t (*perform)(int, void *, Py_ssize_t, Py_UCS4 *, 
Py_UCS4 *))
# {
# PyObject *res = NULL;
# Py_ssize_t length, newlength = 0;
# int kind, outkind;
# (...)
# 1   length = PyUnicode_GET_LENGTH(self);
# 2   tmp = PyMem_MALLOC(sizeof(Py_UCS4) * 3 * length);
# (...)
# 3   newlength = perform(kind, data, length, tmp, maxchar);
# 
# 1. there are no safety checks 
# 2. 12*length overflows
# 3. perform() writes to tmp buffer, which is too small to hold the result

--
files: poc_case_op.py
messages: 229455
nosy: pkt
priority: normal
severity: normal
status: open
title: Integer overflow in case_operation
type: security
versions: Python 3.4
Added file: http://bugs.python.org/file36941/poc_case_op.py

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

[issue22643] Integer overflow in case_operation

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
nosy: +haypo

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

[issue22643] Integer overflow in case_operation

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
components: +Unicode
nosy: +ezio.melotti

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

[issue22643] Integer overflow in case_operation

[Ezio Melotti]

Changes by Ezio Melotti ezio.melo...@gmail.com:


--
nosy: +benjamin.peterson, serhiy.storchaka
type: security - crash

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

[issue22643] Integer overflow in case_operation

[Roundup Robot]

Roundup Robot added the comment:

New changeset 449b1f427cc7 by Benjamin Peterson in branch '3.3':
fix integer overflow in unicode case operations (closes #22643)
https://hg.python.org/cpython/rev/449b1f427cc7

New changeset 570e70252d5d by Benjamin Peterson in branch '3.4':
merge 3.3 (#22643)
https://hg.python.org/cpython/rev/570e70252d5d

New changeset c2980ec10a4c by Benjamin Peterson in branch 'default':
merge 3.4 (#22643)
https://hg.python.org/cpython/rev/c2980ec10a4c

--
nosy: +python-dev
resolution:  - fixed
stage:  - resolved
status: open - closed

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

[issue22643] Integer overflow in case_operation

[STINNER Victor]

STINNER Victor added the comment:

Benjamin, could you please first propose a patch for review instead of 
commiting directly your change? Especially for security related changes.

+if (length  PY_SSIZE_T_MAX / 3 ||
+length  PY_SIZE_MAX / (3 * sizeof(Py_UCS4))) {
+PyErr_SetString(PyExc_OverflowError, string is too long);
+return NULL;
+}
 tmp = PyMem_MALLOC(sizeof(Py_UCS4) * 3 * length);

PyMem_MALLOC() returns NULL if the length is larger than PY_SSIZE_T_MAX, so the 
overflow check doesn't look correct. The overflow check can be replaced with:

if ((size_t)length  PY_SSIZE_T_MAX / (3 * sizeof(Py_UCS4))) ...

--
resolution: fixed - 
status: closed - open

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

[issue22643] Integer overflow in case_operation

[STINNER Victor]

STINNER Victor added the comment:

Other changesets related to this issue:

changeset:   93071:6a91e616485a
branch:  3.3
parent:  93068:449b1f427cc7
user:Benjamin Peterson benja...@python.org
date:Wed Oct 15 11:51:05 2014 -0400
files:   Objects/unicodeobject.c
description:
make sure length is unsigned

(followed by merged into 3.4 and 3.5)

--

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

[issue22643] Integer overflow in case_operation

[Benjamin Peterson]

Benjamin Peterson added the comment:

Cool, I forgot about that.

On Wed, Oct 15, 2014, at 12:11, STINNER Victor wrote:
 
 STINNER Victor added the comment:
 
 Other changesets related to this issue:
 
 changeset:   93071:6a91e616485a
 branch:  3.3
 parent:  93068:449b1f427cc7
 user:Benjamin Peterson benja...@python.org
 date:Wed Oct 15 11:51:05 2014 -0400
 files:   Objects/unicodeobject.c
 description:
 make sure length is unsigned
 
 (followed by merged into 3.4 and 3.5)
 
 --
 
 ___
 Python tracker rep...@bugs.python.org
 http://bugs.python.org/issue22643
 ___

--

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

[issue22643] Integer overflow in case_operation

[Roundup Robot]

Roundup Robot added the comment:

New changeset f963cc1f96cf by Benjamin Peterson in branch '3.3':
it suffices to check for PY_SSIZE_T_MAX overflow (#22643)
https://hg.python.org/cpython/rev/f963cc1f96cf

New changeset 8195d48a5c43 by Benjamin Peterson in branch 'default':
merge 3.4 (closes #22643)
https://hg.python.org/cpython/rev/8195d48a5c43

--
resolution:  - fixed
status: open - closed

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

I think this is a place where _PyUnicodeWriter would be appropriate (of course 
this is different issue).

--

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

The test should be decorated with the bigmemtest decorator. And I think that 
condition sys.maxsize  2**32 would be more robust.

--
status: closed - open

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

[issue22643] Integer overflow in case_operation

[Benjamin Peterson]

Benjamin Peterson added the comment:

It's only 341 MB.

On Wed, Oct 15, 2014, at 13:29, Serhiy Storchaka wrote:
 
 Serhiy Storchaka added the comment:
 
 The test should be decorated with the bigmemtest decorator. And I think
 that condition sys.maxsize  2**32 would be more robust.
 
 --
 status: closed - open
 
 ___
 Python tracker rep...@bugs.python.org
 http://bugs.python.org/issue22643
 ___

--

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

And this test is CPython only. It relies on specific implementation detail. 
After changing current implementation (which inefficiently uses memory) this 
test will be dropped.

--

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

[issue22643] Integer overflow in case_operation

[Roundup Robot]

Roundup Robot added the comment:

New changeset 33290d0dd946 by Benjamin Peterson in branch '3.4':
merge 3.3 (#22643)
https://hg.python.org/cpython/rev/33290d0dd946

New changeset ffabb674140c by Benjamin Peterson in branch 'default':
merge 3.4 (#22643)
https://hg.python.org/cpython/rev/ffabb674140c

--

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

[issue22643] Integer overflow in case_operation

[Benjamin Peterson]

Benjamin Peterson added the comment:

Since the memory requirement is less than 500MB, I don't think it needs a 
bigmemtest decorator.

--
status: open - closed

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

[issue22643] Integer overflow in case_operation

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

 It's only 341 MB.

It's 2**32//12*2 ~ 683 MiB for original string (but I'm not sure that non-
ASCII string is needed for this test) + 2**32 = 4GiB if the test will fail for 
some reason. Some buildbots AFAIK have memory less than 683 MiB.

--

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

[issue22643] Integer overflow in case_operation

[Benjamin Peterson]

Benjamin Peterson added the comment:

It's Latin 1, so the chars only use one byte:

 sys.getsizeof(üü) - sys.getsizeof(ü)
1

On Wed, Oct 15, 2014, at 13:46, Serhiy Storchaka wrote:
 
 Serhiy Storchaka added the comment:
 
  It's only 341 MB.
 
 It's 2**32//12*2 ~ 683 MiB for original string (but I'm not sure that
 non-
 ASCII string is needed for this test) + 2**32 = 4GiB if the test will
 fail for 
 some reason. Some buildbots AFAIK have memory less than 683 MiB.
 
 --
 
 ___
 Python tracker rep...@bugs.python.org
 http://bugs.python.org/issue22643
 ___

--

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

[issue22643] Integer overflow in case_operation

[Benjamin Peterson]

Benjamin Peterson added the comment:

More explicitly:

 sys.getsizeof(ü*(2**32//12 + 1))//1024//1024
341

--

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