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

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
Labels
3.7 expert-unicode interpreter-core type-feature

Comments

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Jun 20, 2017

BPO 30708
Nosy @vstinner, @ezio-melotti, @serhiy-storchaka, @zooba
PRs
  • #2285
  • #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

    @serhiy-storchaka serhiy-storchaka commented Jun 20, 2017

    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 interpreter-core expert-unicode type-feature labels Jun 20, 2017
    @serhiy-storchaka
    Copy link
    Member Author

    @serhiy-storchaka serhiy-storchaka commented Jun 21, 2017

    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

    @serhiy-storchaka serhiy-storchaka commented Jun 24, 2017

    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

    @serhiy-storchaka serhiy-storchaka commented Jun 27, 2017

    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

    @serhiy-storchaka serhiy-storchaka commented Jun 27, 2017

    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

    @serhiy-storchaka serhiy-storchaka commented Jun 27, 2017

    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

    @serhiy-storchaka serhiy-storchaka commented Jun 27, 2017

    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
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 expert-unicode interpreter-core type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    1 participant