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

Improve size validation in _Py_DecodeUTF8Ex #91421

Closed
stoeckmann opened this issue Apr 10, 2022 · 0 comments
Closed

Improve size validation in _Py_DecodeUTF8Ex #91421

stoeckmann opened this issue Apr 10, 2022 · 0 comments

Comments

@stoeckmann
Copy link
Contributor

The size check in _Py_DecodeUTF8Ex can be improved to always check against a constant value without further arithmetic involved. This is already done at other places within the file, e.g. here.

I was curious if this could actually be triggered with a proof of concept by overflowing the check and eventually performing an out of boundary heap access. And in fact, with a very artificial setup, it is possible on a 32 bit system which tries to convert a 2 GB long string:

#include "Python.h"

#include <sys/mman.h>

#include <err.h>
#include <stdlib.h>

int
main(int argc, char *argv[])
{
        char *str;
        size_t wlen;
        wchar_t *program;

        // force UTF-8 mode
        Py_UTF8Mode = 1;
        if ((program = Py_DecodeLocale(argv[0], NULL)) == NULL)
                errx(1, "PyDecodeLocale");
        Py_SetProgramName(program);
        Py_Initialize();

        // try to convert a 2 GB long string
        if ((str = mmap(NULL, (size_t)INT_MAX + 1, PROT_READ | PROT_WRITE,
            MAP_PRIVATE | MAP_ANONYMOUS, -1, 0)) == (void *)-1)
                err(1, "malloc");
        memset(str, 'a', INT_MAX);
        str[INT_MAX] = '\0';
        Py_DecodeLocale(str, &wlen);

        PyMem_RawFree(program);
        return 0;
}

I doubt that this is really reachable with actual code. But at least it is a good showcase that actual arithmetic is left over in the if-check. Let's remove it and save us this possible headache.

PS: Not sure if this is the correct way to create python issues with GitHub now. Let me know if something's missing or wrong!

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Apr 13, 2022
The left-hand side expression of the if-check can be converted to a
constant by the compiler, but the addition on the right-hand side is
performed during runtime.

Move the addition from the right-hand side to the left-hand side by
turning it into a subtraction there. Since the values are known to
be large enough to not turn negative, this is a safe operation.

Prevents a very unlikely integer overflow on 32 bit systems.

Fixes pythonGH-91421.
(cherry picked from commit 0859368)

Co-authored-by: Tobias Stoeckmann <stoeckmann@users.noreply.github.com>
JelleZijlstra pushed a commit that referenced this issue Apr 14, 2022
The left-hand side expression of the if-check can be converted to a
constant by the compiler, but the addition on the right-hand side is
performed during runtime.

Move the addition from the right-hand side to the left-hand side by
turning it into a subtraction there. Since the values are known to
be large enough to not turn negative, this is a safe operation.

Prevents a very unlikely integer overflow on 32 bit systems.

Fixes GH-91421.
(cherry picked from commit 0859368)

Co-authored-by: Tobias Stoeckmann <stoeckmann@users.noreply.github.com>
JelleZijlstra pushed a commit that referenced this issue Apr 14, 2022
The left-hand side expression of the if-check can be converted to a
constant by the compiler, but the addition on the right-hand side is
performed during runtime.

Move the addition from the right-hand side to the left-hand side by
turning it into a subtraction there. Since the values are known to
be large enough to not turn negative, this is a safe operation.

Prevents a very unlikely integer overflow on 32 bit systems.

Fixes GH-91421.
(cherry picked from commit 0859368)

Co-authored-by: Tobias Stoeckmann <stoeckmann@users.noreply.github.com>
hello-adam pushed a commit to hello-adam/cpython that referenced this issue Jun 2, 2022
…) (pythonGH-91493)

The left-hand side expression of the if-check can be converted to a
constant by the compiler, but the addition on the right-hand side is
performed during runtime.

Move the addition from the right-hand side to the left-hand side by
turning it into a subtraction there. Since the values are known to
be large enough to not turn negative, this is a safe operation.

Prevents a very unlikely integer overflow on 32 bit systems.

Fixes pythonGH-91421.
(cherry picked from commit 0859368)

Co-authored-by: Tobias Stoeckmann <stoeckmann@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant