bpo-34087: Fix buffer overflow in int(s) and similar functions #8274

methane · 2018-07-13T13:12:16Z

_PyUnicode_TransformDecimalAndSpaceToASCII() missed trailing NUL char.
It cause buffer overflow in _Py_string_to_number_with_underscores().

This bug is introduced in bpo-31979, 9b6c60c.

https://bugs.python.org/issue34087

_PyUnicode_TransformDecimalAndSpaceToASCII() missed trailing NUL char. It cause buffer overflow in _Py_string_to_number_with_underscores(). This bug is introduced in bpo-31979, 9b6c60c.

serhiy-storchaka

Any chance of adding tests?

methane · 2018-07-13T13:45:03Z

I added assert, but I haven't checked "make test" cause assertion failure yet. Now I left on my PC. Please wait a day. 2018年7月13日(金) 22:31 Serhiy Storchaka <[email protected]>:

…

***@***.**** approved this pull request. Any chance of adding tests? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#8274 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMLqFRu6P6cw1na4VdB7zmOOdSSpI19ks5uGKEXgaJpZM4VO3qA> .

vstinner · 2018-07-13T14:03:24Z

Python/pystrtod.c

@@ -391,6 +391,8 @@ _Py_string_to_number_with_underscores(
    char *dup, *end;
    PyObject *result;

+    assert(s[orig_len] == '\0');


I'm not sure about this assertion. For performance, it would make sense to call _Py_string_to_number_with_underscores() on a sub-string which doesn't end with a NUL character. If you want to add an assertion, please do it at the caller site.

This assertion is fine to me. _Py_string_to_number_with_underscores() is always called with a substring that end with a NUL character (if it doesn't end, we make a copy and call this function with it). And _Py_string_to_number_with_underscores() calls innerfunc() which expects a NUL terminated string.

Oh, the following lines calls strchr() which expects that the string ends with a NUL byte.

In that case, should we also test that strlen(s) == orig_len to block embedded NUL byte?

It is not needed. NUL byte in string is valid.

>>> len("\0\0\0") 3

vstinner · 2018-07-13T14:16:26Z

Python/pystrtod.c

@@ -391,6 +391,8 @@ _Py_string_to_number_with_underscores(
    char *dup, *end;
    PyObject *result;

+    assert(s[orig_len] == '\0');


Oh, the following lines calls strchr() which expects that the string ends with a NUL byte.

In that case, should we also test that strlen(s) == orig_len to block embedded NUL byte?

vstinner · 2018-07-13T14:17:11Z

Objects/unicodeobject.c

@@ -9072,6 +9072,7 @@ _PyUnicode_TransformDecimalAndSpaceToASCII(PyObject *unicode)
            int decimal = Py_UNICODE_TODECIMAL(ch);
            if (decimal < 0) {
                out[i] = '?';
+                out[i+1] = '\0';
                _PyUnicode_LENGTH(result) = i + 1;
                break;
            }


To debug other kinds of bugs, you can call "assert(_PyUnicode_CheckConsistency(unicode, 1));" just before "return result;". This function detects if the last character is not a NUL character, for example ;-)

vstinner

LGTM.

I tested manually and confirm that the 3 tests (test_complex test_float test_long) crash without the fix ("out[i+1] = '\0';"), and pass with the fix.

vstinner · 2018-07-13T15:55:18Z

You might backport the new tests to 3.6 (it's up to you).

miss-islington · 2018-07-14T03:06:46Z

Thanks @methane for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

bedevere-bot · 2018-07-14T03:07:18Z

GH-8279 is a backport of this pull request to the 3.7 branch.

…nGH-8274) `_PyUnicode_TransformDecimalAndSpaceToASCII()` missed trailing NUL char. It caused buffer overflow in `_Py_string_to_number_with_underscores()`. This bug is introduced in 9b6c60c. (cherry picked from commit 16dfca4) Co-authored-by: INADA Naoki <[email protected]>

`_PyUnicode_TransformDecimalAndSpaceToASCII()` missed trailing NUL char. It caused buffer overflow in `_Py_string_to_number_with_underscores()`. This bug is introduced in 9b6c60c. (cherry picked from commit 16dfca4) Co-authored-by: INADA Naoki <[email protected]>

pythonGH-8276 fixed regression in 3.7. While the regression is not in 3.6, it's worth to backport test cases to 3.6 branch too.

Cherrypick tests from 16dfca4 While the regression is not in 3.6, it's worth to backport test cases to 3.6 branch too.

* master: (2633 commits) bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274) bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271) bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924) bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267) bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261) bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222) Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887) bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192) bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114) bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245) Simplify __all__ in multiprocessing (pythonGH-6856) bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230) Doc: Point to Simple statements section instead of PEP (pythonGH-8238) bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139) bpo-33597: Add What's New for PyGC_Head (pythonGH-8236) Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038) Fix documentation for input and output tutorial (pythonGH-8231) bpo-34009: Expand on platform support changes (pythonGH-8022) Factor-out two substantially identical code blocks. (pythonGH-8219) bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091) ...

* master: (1159 commits) bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274) bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271) bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924) bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267) bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261) bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222) Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887) bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192) bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114) bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245) Simplify __all__ in multiprocessing (pythonGH-6856) bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230) Doc: Point to Simple statements section instead of PEP (pythonGH-8238) bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139) bpo-33597: Add What's New for PyGC_Head (pythonGH-8236) Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038) Fix documentation for input and output tutorial (pythonGH-8231) bpo-34009: Expand on platform support changes (pythonGH-8022) Factor-out two substantially identical code blocks. (pythonGH-8219) bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091) ...

…ssue-33014 * 'master' of github.com:CuriousLearner/cpython: (722 commits) bpo-34087: Fix buffer overflow in int(s) and similar functions (pythonGH-8274) bpo-34108: Fix double carriage return in 2to3 on Windows (python#8271) bpo-4260: Document that ctypes.xFUNCTYPE are decorators (pythonGH-7924) bpo-33723: Fix test_time.test_thread_time() (pythonGH-8267) bpo-33967: Remove use of deprecated assertRaisesRegexp() (pythonGH-8261) bpo-34080: Fix a memory leak in the compiler. (pythonGH-8222) Enable GUI testing on Travis Linux builds via Xvfb (pythonGH-7887) bpo-23927: Make getargs.c skipitem() skipping 'w*'. (pythonGH-8192) bpo-33648: Remove PY_WARN_ON_C_LOCALE (pythonGH-7114) bpo-34092, test_logging: increase SMTPHandlerTest timeout (pythonGH-8245) Simplify __all__ in multiprocessing (pythonGH-6856) bpo-34083: Update dict order in Functional HOWTO (pythonGH-8230) Doc: Point to Simple statements section instead of PEP (pythonGH-8238) bpo-29442: Replace optparse with argparse in setup.py (pythonGH-139) bpo-33597: Add What's New for PyGC_Head (pythonGH-8236) Dataclasses: Fix example on 30.6.8, add method should receive a list rather than an integer. (pythonGH-8038) Fix documentation for input and output tutorial (pythonGH-8231) bpo-34009: Expand on platform support changes (pythonGH-8022) Factor-out two substantially identical code blocks. (pythonGH-8219) bpo-34031: fix incorrect usage of self.fail in two tests (pythonGH-8091) ...

methane added 2 commits July 13, 2018 22:03

Fix int(s) and similar function may break memory

dfd63aa

_PyUnicode_TransformDecimalAndSpaceToASCII() missed trailing NUL char. It cause buffer overflow in _Py_string_to_number_with_underscores(). This bug is introduced in bpo-31979, 9b6c60c.

Add NEWS

46d4225

methane added type-bug An unexpected behavior, bug, or error needs backport to 3.7 labels Jul 13, 2018

the-knights-who-say-ni added the CLA signed label Jul 13, 2018

bedevere-bot added the awaiting merge label Jul 13, 2018

methane requested review from vstinner and serhiy-storchaka July 13, 2018 13:12

serhiy-storchaka approved these changes Jul 13, 2018

View reviewed changes

vstinner reviewed Jul 13, 2018

View reviewed changes

methane added 2 commits July 14, 2018 00:22

Add one more assert

938eb5e

Add tests causing assertion failure

85d976f

vstinner approved these changes Jul 13, 2018

View reviewed changes

methane merged commit 16dfca4 into python:master Jul 14, 2018

bedevere-bot removed the awaiting merge label Jul 14, 2018

methane deleted the missing-nul branch July 14, 2018 03:06

bedevere-bot removed the needs backport to 3.7 label Jul 14, 2018

methane added a commit to methane/cpython that referenced this pull request Jul 14, 2018

bpo-34087: Backport test added by (pythonGH-8274)

0fb174a

pythonGH-8276 fixed regression in 3.7. While the regression is not in 3.6, it's worth to backport test cases to 3.6 branch too.

methane added a commit that referenced this pull request Jul 14, 2018

bpo-34087: Backport tests for int/float/complex (GH-8274)

b2f8aa0

Cherrypick tests from 16dfca4 While the regression is not in 3.6, it's worth to backport test cases to 3.6 branch too.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

bpo-34087: Fix buffer overflow in int(s) and similar functions #8274

bpo-34087: Fix buffer overflow in int(s) and similar functions #8274

Uh oh!

methane commented Jul 13, 2018 •

edited by bedevere-bot

Loading

Uh oh!

serhiy-storchaka left a comment

Uh oh!

methane commented Jul 13, 2018 via email

Uh oh!

vstinner Jul 13, 2018

Uh oh!

serhiy-storchaka Jul 13, 2018

Uh oh!

vstinner Jul 13, 2018

Uh oh!

methane Jul 13, 2018

Uh oh!

vstinner Jul 13, 2018

Uh oh!

vstinner Jul 13, 2018

Uh oh!

vstinner left a comment

Uh oh!

vstinner commented Jul 13, 2018

Uh oh!

miss-islington commented Jul 14, 2018

Uh oh!

bedevere-bot commented Jul 14, 2018

Uh oh!

Uh oh!

Uh oh!

bpo-34087: Fix buffer overflow in int(s) and similar functions #8274

bpo-34087: Fix buffer overflow in int(s) and similar functions #8274

Uh oh!

Conversation

methane commented Jul 13, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

methane commented Jul 13, 2018 via email

Uh oh!

vstinner Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

methane Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner Jul 13, 2018

Choose a reason for hiding this comment

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Jul 13, 2018

Uh oh!

miss-islington commented Jul 14, 2018

Uh oh!

bedevere-bot commented Jul 14, 2018

Uh oh!

Uh oh!

methane commented Jul 13, 2018 •

edited by bedevere-bot

Loading