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-38876: Raise pickle.UnpicklingError when loading an item from memo for invalid input #17335

Merged

Conversation

PCManticore
Copy link
Contributor

@PCManticore PCManticore commented Nov 22, 2019

The previous code was raising a KeyError for both the Python and C implementation.
This was caused by the specified index of an invalid input which did not exist
in the memo structure, where the pickle stores what objects it has seen.
The malformed input would have caused either a BINGET or LONG_BINGET load
from the memo, leading to a KeyError as the determined index was bogus.

https://bugs.python.org/issue38876

https://bugs.python.org/issue38876

Automerge-Triggered-By: @avassalotti

PyErr_SetObject(PyExc_KeyError, key);
if (!PyErr_Occurred()) {
PickleState *st = _Pickle_GetGlobalState();
PyErr_Format(st->UnpicklingError, "Memo value not found at index");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you show the invalid memo key in the exception message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing, let me know if the new changes make sense.

…o for invalid input

The previous code was raising a `KeyError` for both the Python and C implementation.
This was caused by the specified index of an invalid input which did not exist
in the memo structure, where the pickle stores what objects it has seen.
The malformed input would have caused either a `BINGET` or `LONG_BINGET` load
from the memo, leading to a `KeyError` as the determined index was bogus.
@PCManticore PCManticore force-pushed the bpo-38876-change-pickle-error branch from 15a30cc to f8e15c1 Compare November 23, 2019 08:39
@PCManticore
Copy link
Contributor Author

Thank you for the review @avassalotti ! Let me know if the latest patch make sense.

@miss-islington miss-islington merged commit 6f03b23 into python:master Nov 24, 2019
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
…o for invalid input (pythonGH-17335)

The previous code was raising a `KeyError` for both the Python and C implementation.
This was caused by the specified index of an invalid input which did not exist
in the memo structure, where the pickle stores what objects it has seen.
The malformed input would have caused either a `BINGET` or `LONG_BINGET` load
from the memo, leading to a `KeyError` as the determined index was bogus.

https://bugs.python.org/issue38876



https://bugs.python.org/issue38876
PyErr_SetObject(PyExc_KeyError, key);
if (!PyErr_Occurred()) {
PickleState *st = _Pickle_GetGlobalState();
PyErr_Format(st->UnpicklingError, "Memo value not found at index %ld", idx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

%zd should be used with Py_ssize_t, not %ld.

@@ -6201,7 +6203,8 @@ load_binget(UnpicklerObject *self)
if (value == NULL) {
PyObject *key = PyLong_FromSsize_t(idx);
if (key != NULL) {
PyErr_SetObject(PyExc_KeyError, key);
PickleState *st = _Pickle_GetGlobalState();
PyErr_Format(st->UnpicklingError, "Memo value not found at index %ld", idx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@@ -6227,7 +6230,8 @@ load_long_binget(UnpicklerObject *self)
if (value == NULL) {
PyObject *key = PyLong_FromSsize_t(idx);
if (key != NULL) {
PyErr_SetObject(PyExc_KeyError, key);
PickleState *st = _Pickle_GetGlobalState();
PyErr_Format(st->UnpicklingError, "Memo value not found at index %ld", idx);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

shihai1991 pushed a commit to shihai1991/cpython that referenced this pull request Jan 31, 2020
…o for invalid input (pythonGH-17335)

The previous code was raising a `KeyError` for both the Python and C implementation.
This was caused by the specified index of an invalid input which did not exist
in the memo structure, where the pickle stores what objects it has seen.
The malformed input would have caused either a `BINGET` or `LONG_BINGET` load
from the memo, leading to a `KeyError` as the determined index was bogus.

https://bugs.python.org/issue38876



https://bugs.python.org/issue38876
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

Successfully merging this pull request may close these issues.

6 participants