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.

Sign up for GitHub

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

Jump to bottom

Ensure that the result of PyUnicode_AsWideCharString() doesn't contain null characters if size is not returned #74893

Closed
serhiy-storchaka opened this issue Jun 20, 2017 · 7 comments
Closed

Ensure that the result of PyUnicode_AsWideCharString() doesn't contain null characters if size is not returned #74893

serhiy-storchaka opened this issue Jun 20, 2017 · 7 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-unicode type-feature A feature request or enhancement

Comments

@serhiy-storchaka
Copy link
Member

BPO 30708
Nosy @vstinner, @ezio-melotti, @serhiy-storchaka, @zooba
PRs
  • bpo-30708: Check for null characters in PyUnicode_AsWideCharString(). #2285
  • [3.6] bpo-30708: Add private C API function _PyUnicode_AsWideCharString(). (GH-2285) #2443
  • [3.5] bpo-30708: Add private C API function _PyUnicode_AsWideCharString(). (GH-2285) (GH-2443) #2448
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2017-06-27.19:06:14.376>
created_at = <Date 2017-06-20.04:38:10.522>
labels = ['interpreter-core', 'type-feature', '3.7', 'expert-unicode']
title = "Ensure that the result of PyUnicode_AsWideCharString() doesn't contain null characters if size is not returned"
updated_at = <Date 2017-06-27.19:06:14.374>
user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2017-06-27.19:06:14.374>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2017-06-27.19:06:14.376>
closer = 'serhiy.storchaka'
components = ['Interpreter Core', 'Unicode']
creation = <Date 2017-06-20.04:38:10.522>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 30708
keywords = []
message_count = 7.0
messages = ['296401', '296514', '296755', '297031', '297062', '297066', '297069']
nosy_count = 4.0
nosy_names = ['vstinner', 'ezio.melotti', 'serhiy.storchaka', 'steve.dower']
pr_nums = ['2285', '2443', '2448']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue30708'
versions = ['Python 3.7']
    @serhiy-storchaka
    Copy link
    Member Author

    The second parameter of the PyUnicode_AsWideCharString() function

    wchar_t* PyUnicode_AsWideCharString(PyObject *unicode, Py_ssize_t *size)

    is a pointer to Py_ssize_t. The size of created wchar_t array is saved on this pointer if it is not NULL. If NULL is passed as the second argument, the only way to determine the size of the wchar_t string is using wcslen(). But if the string contains the null characters, it looks truncated for wcslen() and other C API functions.

    Reliable code should always pass the non-NULL second argument and check that wcslen() is equal to the returned string size. See for example the code in Modules/_io/winconsoleio.c. Passing NULL as the second argument is unsafe. But most code doesn't do such check (see all other usages of PyUnicode_AsWideCharString(..., NULL)). And this check complicates the callers code.

    I propose to make the check for null characters inside of PyUnicode_AsWideCharString() if NULL is passes as the second argument. This will fix all unsafe usages of PyUnicode_AsWideCharString() and allow to simplify the reliable code.

    This issue fixes the part of bpo-13617.

    @serhiy-storchaka serhiy-storchaka added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-unicode type-feature A feature request or enhancement labels Jun 20, 2017
    @serhiy-storchaka
    Copy link
    Member Author

    This change needs changing one ctypes test, and can break third-party tests or even a code. That is why it is targeted only for 3.7. I'm going to backport the change as a private function for using in CPython internally since this can fix vulnerabilities.

    @serhiy-storchaka
    Copy link
    Member Author

    Could anyone please make a review of this PR? Especially the documentation part. This PR is a part of a set of PRs that fix potential vulnerabilities (bpo-13617, bpo-30730, and yet few issues planned).

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset e613e6a by Serhiy Storchaka in branch 'master':
    bpo-30708: Check for null characters in PyUnicode_AsWideCharString(). (bpo-2285)
    e613e6a

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 0edffa3 by Serhiy Storchaka in branch '3.6':
    [3.6] bpo-30708: Check for null characters in PyUnicode_AsWideCharString(). (GH-2285) (bpo-2443)
    0edffa3

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 94b169f by Serhiy Storchaka in branch '3.5':
    [3.5] bpo-30708: Add private C API function _PyUnicode_AsWideCharString(). (GH-2285) (GH-2443) (bpo-2448)
    94b169f

    @serhiy-storchaka
    Copy link
    Member Author

    Wrong commit message in 3.6. Should be the same as in 3.5.

    This functionality was backported as a private function _PyUnicode_AsWideCharString().

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @serhiy-storchaka serhiy-storchaka mentioned this issue Nov 3, 2023
    Closed
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-unicode type-feature A feature request or enhancement
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @serhiy-storchaka