Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-35214: Fix OOB memory access in unicode escape parser #10506

Merged
merged 3 commits into from
Nov 13, 2018

Conversation

gpshead
Copy link
Member

@gpshead gpshead commented Nov 13, 2018

Discovered using clang's MemorySanitizer when it ran
test_fstring's test_misformed_unicode_character_name.

An f-string ending in \N would access one byte beyond the end of
the string while looking for a potential }.

https://bugs.python.org/issue35214

Discovered using clang's MemorySanitizer when it ran
test_fstring's test_misformed_unicode_character_name.

An f-string ending in `\N` would access one byte beyond the end of
the string while looking for a potential `}`.
@gpshead gpshead added type-bug An unexpected behavior, bug, or error needs backport to 3.6 labels Nov 13, 2018
@gpshead gpshead self-assigned this Nov 13, 2018
@gpshead
Copy link
Member Author

gpshead commented Nov 13, 2018

actually my news entry and description may not be correct. this may not be specific to f-strings but to any unicode string escape parsing?

@gpshead
Copy link
Member Author

gpshead commented Nov 13, 2018

Confirmed, this is in the unicode parser, not f-string related. that just happened to be the unittest that triggered it.

$ ./python -c 'u"\N"'
==17380==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x6a8d8f in _PyUnicode_DecodeUnicodeEscape /home/gps/test/b37/../cpython/Objects/unicodeobject.c:6045:17
    #1 0xbcd1c8 in decode_unicode_with_escapes /home/gps/test/b37/../cpython/Python/ast.c:4208:9
    #2 0xbcaf2c in parsestr /home/gps/test/b37/../cpython/Python/ast.c:5175:23
    #3 0xbc8be3 in parsestrplus /home/gps/test/b37/../cpython/Python/ast.c:5206:13
    #4 0xbc6516 in ast_for_atom /home/gps/test/b37/../cpython/Python/ast.c:2107:23
    #5 0xbc57af in ast_for_atom_expr /home/gps/test/b37/../cpython/Python/ast.c:2464:9
    #6 0xbc1796 in ast_for_power /home/gps/test/b37/../cpython/Python/ast.c:2501:9
    #7 0xbbec1d in ast_for_expr /home/gps/test/b37/../cpython/Python/ast.c:2689:20
    #8 0xbb99a4 in ast_for_testlist /home/gps/test/b37/../cpython/Python/ast.c:2878:16
    #9 0xbd606e in ast_for_expr_stmt /home/gps/test/b37/../cpython/Python/ast.c:2901:21
    #10 0xbb92cf in ast_for_stmt /home/gps/test/b37/../cpython/Python/ast.c:3983:24
    #11 0xbb820a in PyAST_FromNodeObject /home/gps/test/b37/../cpython/Python/ast.c:799:25
    #12 0x88d4cb in PyParser_ASTFromStringObject /home/gps/test/b37/../cpython/Python/pythonrun.c:1184:15
    #13 0x889f61 in PyRun_StringFlags /home/gps/test/b37/../cpython/Python/pythonrun.c:957:11
    #14 0x889c33 in PyRun_SimpleStringFlags /home/gps/test/b37/../cpython/Python/pythonrun.c:455:9
...

@gpshead gpshead changed the title bpo-35214: Fix OOB memory access in f-string parser bpo-35214: Fix OOB memory access in unicode escape parser Nov 13, 2018
@gpshead
Copy link
Member Author

gpshead commented Nov 13, 2018

In terms of severity, this was only ever a single byte read. The code path it entered if that byte happened to be a } did check the bounds and would result in the desired error message (which is why this went unnoticed for so long). With most memory allocations it is unlikely that this byte would be on an unmapped page and crash the program, though I suspect it would be possible to trigger that by using a large enough string to generate such a specific allocation.

At most that is a crash of an interpreter from an errant memory read. Denial of service at best if someone is handing user data to the unicode escape parser.

@gpshead gpshead merged commit 746b2d3 into python:master Nov 13, 2018
@miss-islington
Copy link
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 13, 2018
…0506)

Discovered using clang's MemorySanitizer when it ran python3's
test_fstring test_misformed_unicode_character_name.

An msan build will fail by simply executing: ./python -c 'u"\N"'
(cherry picked from commit 746b2d3)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
@bedevere-bot
Copy link

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

@bedevere-bot
Copy link

GH-10523 is a backport of this pull request to the 3.6 branch.

@miss-islington
Copy link
Contributor

Sorry, @gpshead, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 746b2d35ea47005054ed774fecaed64fab803d7d 2.7

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Nov 13, 2018
…0506)

Discovered using clang's MemorySanitizer when it ran python3's
test_fstring test_misformed_unicode_character_name.

An msan build will fail by simply executing: ./python -c 'u"\N"'
(cherry picked from commit 746b2d3)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
@gpshead gpshead deleted the msan_fstring_oob branch November 13, 2018 21:18
miss-islington added a commit that referenced this pull request Nov 13, 2018
Discovered using clang's MemorySanitizer when it ran python3's
test_fstring test_misformed_unicode_character_name.

An msan build will fail by simply executing: ./python -c 'u"\N"'
(cherry picked from commit 746b2d3)

Co-authored-by: Gregory P. Smith <greg@krypto.org>
miss-islington added a commit that referenced this pull request Nov 14, 2018
…0506) (GH-10522)

Discovered using clang's MemorySanitizer when it ran python3's
test_fstring test_misformed_unicode_character_name.

An msan build will fail by simply executing: ./python -c 'u"\N"'
(cherry picked from commit 746b2d3)


Co-authored-by: Gregory P. Smith <greg@krypto.org>


https://bugs.python.org/issue35214
@bedevere-bot
Copy link

GH-10538 is a backport of this pull request to the 2.7 branch.

gpshead added a commit that referenced this pull request Nov 14, 2018
…0506) (GH-10538)

Discovered using clang's MemorySanitizer.

A msan build will fail by simply executing: ./python -c 'u"\N"'
(cherry picked from commit 746b2d3)

Co-authored-by: Gregory P. Smith <greg@krypto.org> [Google LLC]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants