Skip to content

_imp.get_frozen_object possible incorrect use of PyBUF_READ #114685

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
Closed
sobolevn opened this issue Jan 28, 2024 · 6 comments
Closed

_imp.get_frozen_object possible incorrect use of PyBUF_READ #114685

sobolevn opened this issue Jan 28, 2024 · 6 comments
Assignees
@sobolevn
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Jan 28, 2024 •
edited by bedevere-app bot
Loading

Bug report

It is documented that PyBUF_READ should be used with memoryview objects. But, it is used in PyObject_GetBuffer:

cpython/Python/import.c

Line 3547 in 5ecfd75

if (PyObject_GetBuffer(dataobj, &buf, PyBUF_READ) != 0) {

Other similar places that access .buf and .len just use PyBUF_SIMPLE, which I think we should use here as well.

I will send a PR.

Originally found by @serhiy-storchaka in #114669 (comment)

Linked PRs
@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Jan 28, 2024
@sobolevn sobolevn self-assigned this Jan 28, 2024
sobolevn added a commit to sobolevn/cpython that referenced this issue Jan 28, 2024
@sobolevn
pythongh-114685: Fix incorrect use of PyBUF_READ in import.c
3c0c393
@bedevere-app bedevere-app bot mentioned this issue Jan 28, 2024
Merged
@serhiy-storchaka
Copy link
Member

Other question, why it was not caught in tests? Does PyObject_GetBuffer() ignore unknown flags or this code was never executed?

@sobolevn
Copy link
Member Author

PyObject_GetBuffer() expects a numeric flag, but it uses bitflag model with example logic:

    if (!REQ_FORMAT(flags)) {
        /* NULL indicates that the buffer's data type has been cast to 'B'.
           view->itemsize is the _previous_ itemsize. If shape is present,
           the equality product(shape) * itemsize = len still holds at this
           point. The equality calcsize(format) = itemsize does _not_ hold
           from here on! */
        view->format = NULL;
    }
    if (REQ_C_CONTIGUOUS(flags) && !MV_C_CONTIGUOUS(baseflags)) {
        PyErr_SetString(PyExc_BufferError,
            "memoryview: underlying buffer is not C-contiguous");
        return -1;
    }
    if (REQ_F_CONTIGUOUS(flags) && !MV_F_CONTIGUOUS(baseflags)) {
        PyErr_SetString(PyExc_BufferError,
            "memoryview: underlying buffer is not Fortran contiguous");
        return -1;
    }
    if (REQ_ANY_CONTIGUOUS(flags) && !MV_ANY_CONTIGUOUS(baseflags)) {
        PyErr_SetString(PyExc_BufferError,
            "memoryview: underlying buffer is not contiguous");
        return -1;
    }

PyBYF_READ is equal to 256, so it just contains all the features. PyBUF_SIMPLE is just 0, so no extras.

serhiy-storchaka pushed a commit that referenced this issue Jan 29, 2024
@sobolevn
gh-114685: Fix incorrect use of PyBUF_READ in import.c (GH-114686)
1ac1b2f
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 29, 2024
@sobolevn @miss-islington
pythongh-114685: Fix incorrect use of PyBUF_READ in import.c (pythonG…
6fecdaf 
…H-114686)

(cherry picked from commit 1ac1b2f)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jan 29, 2024
@sobolevn @miss-islington
pythongh-114685: Fix incorrect use of PyBUF_READ in import.c (pythonG…
432bfcf 
…H-114686)

(cherry picked from commit 1ac1b2f)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
This was referenced Jan 29, 2024
Merged
Merged
serhiy-storchaka pushed a commit that referenced this issue Jan 29, 2024
@miss-islington @sobolevn
[3.11] gh-114685: Fix incorrect use of PyBUF_READ in import.c (GH-114686
ee3ca96 
) (GH-114701)

(cherry picked from commit 1ac1b2f)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
serhiy-storchaka pushed a commit that referenced this issue Jan 29, 2024
@miss-islington @sobolevn
[3.12] gh-114685: Fix incorrect use of PyBUF_READ in import.c (GH-114686
2a1d2c8 
) (GH-114700)

(cherry picked from commit 1ac1b2f)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Jan 29, 2024
@serhiy-storchaka
pythongh-114685: Check flags in PyObject_GetBuffer()
86998f2 
PyObject_GetBuffer() now raises a SystemError if called with
PyBUF_READ or PyBUF_WRITE as flags. These flags should
only be used with the PyMemoryView_* C API.
@bedevere-app bedevere-app bot mentioned this issue Jan 29, 2024
Merged
serhiy-storchaka added a commit that referenced this issue Jan 31, 2024
@serhiy-storchaka
gh-114685: Check flags in PyObject_GetBuffer() (GH-114707)
b7688ef 
PyObject_GetBuffer() now raises a SystemError if called with
PyBUF_READ or PyBUF_WRITE as flags. These flags should
only be used with the PyMemoryView_* C API.
@serhiy-storchaka
Copy link
Member

Unfortunately the value of PyBUF_READ intersects with PyBUF_INDIRECT, PyBUF_FULL and PyBUF_FULL_RO, so we cannot just ban this bit. But fortunately other values has other bits set, so this error is still distinguishable.

@sobolevn
Copy link
Member Author

What about PyBuffer_FillInfo(Py_buffer *view, PyObject *obj, void *buf, Py_ssize_t len, int readonly, int flags)? Do we need to check flags there as well?

@serhiy-storchaka
Copy link
Member

Maybe. We do not have examples of the misuse in this case, and it is much less used, but a similar error is possible here too.

@sobolevn
Copy link
Member Author

I will send an example PR, so we can decide 👍

sobolevn added a commit to sobolevn/cpython that referenced this issue Jan 31, 2024
@sobolevn
pythongh-114685: PyBuffer_FillInfo now raises on PyBUF_{READ,WRITE}
350a6e7
@bedevere-app bedevere-app bot mentioned this issue Jan 31, 2024
Merged
sobolevn added a commit to sobolevn/cpython that referenced this issue Jan 31, 2024
@sobolevn
pythongh-114685: PyBuffer_FillInfo now raises on PyBUF_{READ,WRITE}
7e480af
serhiy-storchaka pushed a commit that referenced this issue Feb 4, 2024
@sobolevn
gh-114685: PyBuffer_FillInfo() now raises on PyBUF_{READ,WRITE} (GH-1…
929d44e 
…14802)
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@sobolevn @aisk
pythongh-114685: Fix incorrect use of PyBUF_READ in import.c (pythonG…
27019bc 
…H-114686)
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@serhiy-storchaka @aisk
pythongh-114685: Check flags in PyObject_GetBuffer() (pythonGH-114707)
2fcbcf0 
PyObject_GetBuffer() now raises a SystemError if called with
PyBUF_READ or PyBUF_WRITE as flags. These flags should
only be used with the PyMemoryView_* C API.
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
@sobolevn @aisk
pythongh-114685: PyBuffer_FillInfo() now raises on PyBUF_{READ,WRITE} (…
4db9b53 
…pythonGH-114802)
fsc-eriker pushed a commit to fsc-eriker/cpython that referenced this issue Feb 14, 2024
@sobolevn @fsc-eriker
pythongh-114685: PyBuffer_FillInfo() now raises on PyBUF_{READ,WRITE} (…
a97e434 
…pythonGH-114802)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sobolevn sobolevn

Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@serhiy-storchaka @sobolevn