Issue 22518: integer overflow in encoding unicode

# static PyObject *
# unicode_encode_ucs1(PyObject *unicode,
#                     const char *errors,
#                     unsigned int limit)
# {
#     ...
#     while (pos < size) {
#       ...
#             case 4: /* xmlcharrefreplace */
#                 /* determine replacement size */
#                 for (i = collstart, repsize = 0; i < collend; ++i) {
#                     Py_UCS4 ch = PyUnicode_READ(kind, data, i);
#                     ...
#                     else if (ch < 100000)
# 1                       repsize += 2+5+1;
#                     ...
#                 }
# 2               requiredsize = respos+repsize+(size-collend);
#                 if (requiredsize > ressize) {
#                     ...
#                     if (_PyBytes_Resize(&res, requiredsize))
#                     ...
#                 }
#                 /* generate replacement */
#                 for (i = collstart; i < collend; ++i) {
# 3                   str += sprintf(str, "&#%d;", PyUnicode_READ(kind, data, i)); 
#                 }
# 
# 1. ch=0xffff<100000, so repsize = (number of unicode chars in string)*8
#    =2^29*2^3=2^32 == 0 (mod 2^32)
# 2. respos==0, collend==0, so requiredsize=repsize==0, so the destination buffer
#    isn't resized
# 3. overwrite

Looks very similar to issue22470.

New changeset b2e68274aa8e by Benjamin Peterson in branch '2.7':
cleanup overflowing handling in unicode_decode_call_errorhandler and unicode_encode_ucs1 (closes #22518)
https://hg.python.org/cpython/rev/b2e68274aa8e

New changeset 3b7e93249700 by Benjamin Peterson in branch '2.7':
add NEWS note for #22518
https://hg.python.org/cpython/rev/3b7e93249700

New changeset 3c67d19c624f by Benjamin Peterson in branch '3.3':
cleanup overflowing handling in unicode_decode_call_errorhandler and unicode_encode_ucs1 (closes #22518)
https://hg.python.org/cpython/rev/3c67d19c624f

New changeset 88332ea4c140 by Benjamin Peterson in branch '3.3':
NEWS issue for #22518
https://hg.python.org/cpython/rev/88332ea4c140

New changeset 7dab27fffff2 by Benjamin Peterson in branch '3.4':
merge 3.3 (closes #22518)
https://hg.python.org/cpython/rev/7dab27fffff2

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

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

This changeset added ">>>> other". It looks like you commited a conflict.

-        if (requiredsize<2*outsize)
+        if (outsize <= PY_SSIZE_T_MAX/2 && requiredsize < 2*outsize)
             requiredsize = 2*outsize;

I'm not sure that this change is correct. Why not raising an exception on overflow?

It would be nice to add a bigmem test to check that repr('\x00'*(2**30+1)) doesn't crash anymore.

> It would be nice to add a bigmem test to check that repr('\x00'*(2**30+1)) doesn't crash anymore.

Ooops, wrong issue, the test is : ("\uffff" * (2**29)).encode("latin1", errors="xmlcharrefreplace").

> I'm not sure that this change is correct. Why not raising an exception on
> overflow?

This is correct. This check prevents overflow.

> This is correct. This check prevents overflow.

Oh, I didn't understand that "requiredsize = 2*outsize;" is only used for performances, to overallocate the buffer. So I agree that it's fine to not overallocate if it would overflow.

Benjamin, could you make a patch for 3.2 as well?

> Ooops, wrong issue, the test is : ("\uffff" * (2**29)).encode("latin1",
> errors="xmlcharrefreplace").

("\uffff" * (sys.maxsize//8+1)).encode("latin1", errors="xmlcharrefreplace")

or

("\xff" * (sys.maxsize//6+1)).encode("ascii", errors="xmlcharrefreplace")

New changeset 3f7519f633ed by Serhiy Storchaka in branch '2.7':
Issue #22518: Fixed integer overflow issues in "backslashreplace" and
https://hg.python.org/cpython/rev/3f7519f633ed

New changeset ec9b7fd246b6 by Serhiy Storchaka in branch '3.4':
Issue #22518: Fixed integer overflow issues in "backslashreplace",
https://hg.python.org/cpython/rev/ec9b7fd246b6

New changeset 2df4cc31c36e by Serhiy Storchaka in branch 'default':
Issue #22518: Fixed integer overflow issues in "backslashreplace",
https://hg.python.org/cpython/rev/2df4cc31c36e

New changeset d1be1f355f59 by Serhiy Storchaka in branch '2.7':
Fixed compilation error introduced in 3f7519f633ed (issue #22518).
https://hg.python.org/cpython/rev/d1be1f355f59

New changeset 51317c9786f5 by Serhiy Storchaka in branch '3.3':
Issue #22518: Fixed integer overflow issues in "backslashreplace",
https://hg.python.org/cpython/rev/51317c9786f5

Sorry for noise, these changes are related to issue22470.

Do you want to add a bigmem test or close this issue Benjamin?

I wouldn't object if you had a patch.

Integer overflow errors were fixed in 4 error handlers: surrogatepass, backslashreplace, namereplace, and xmlcharrefreplace. Is is hard to write general robust tests. In worst cases they requires more than sys.maxsize or even sys.maxsize*2 memory and for sure fail with MemoryError. It is possible to write a test for xmlcharrefreplace, but it will not robust, after changes of implementation details it could raise MemoryError instead of OverflowError after consuming all address space.

So I suggest close this issue without tests. Such tests are useless.

nosy: + python-dev
messages: + msg227853

resolution: fixed
stage: resolved