Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-143379: fix UAF in struct.Struct.pack() when object modified by dunder methods#143382
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
a1c2603716e28ce611953ab62167b2079e2369c633444310160ab4b7File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Fix use-after-free in :meth:`struct.Struct.pack` when the | ||
| :class:`struct.Struct` object is mutated by dunder methods (like | ||
| :meth:`object.__bool__`) during packing of arguments. Now this trigger | ||
| :exc:`RuntimeError`. Patch by Sergey B Kirpichev. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -70,6 +70,7 @@ typedef struct{ | ||
| formatcode *s_codes; | ||
| PyObject *s_format; | ||
| PyObject *weakreflist; /* List of weak references */ | ||
| Py_ssize_t mutex_cnt; /* to prevent mutation during packing */ | ||
| } PyStructObject; | ||
| #define PyStructObject_CAST(op) ((PyStructObject *)(op)) | ||
| @@ -1773,6 +1774,7 @@ s_new(PyTypeObject *type, PyObject *args, PyObject *kwds) | ||
| s->s_codes = NULL; | ||
| s->s_size = -1; | ||
| s->s_len = -1; | ||
| s->mutex_cnt = 0; | ||
| } | ||
| return self; | ||
| } | ||
| @@ -1816,6 +1818,11 @@ Struct___init___impl(PyStructObject *self, PyObject *format) | ||
| Py_SETREF(self->s_format, format); | ||
| if (FT_ATOMIC_LOAD_SSIZE(self->mutex_cnt)){ | ||
| PyErr_SetString(PyExc_RuntimeError, | ||
| "Call Struct.__init__() in struct.pack()"); | ||
| return -1; | ||
| } | ||
| ret = prepare_s(self); | ||
| return ret; | ||
| } | ||
| @@ -2139,7 +2146,7 @@ Struct_iter_unpack_impl(PyStructObject *self, PyObject *buffer) | ||
| * argument for where to start processing the arguments for packing, and a | ||
| * character buffer for writing the packed string. The caller must insure | ||
| * that the buffer may contain the required length for packing the arguments. | ||
| * 0 is returned on success, 1 is returned if there is an error. | ||
| * 0 is returned on success, -1 is returned if there is an error. | ||
| * | ||
| */ | ||
| static int | ||
| @@ -2259,10 +2266,15 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) | ||
| char *buf = PyBytesWriter_GetData(writer); | ||
| /* Call the guts */ | ||
| Py_ssize_t prev_cnt = FT_ATOMIC_LOAD_SSIZE(soself->mutex_cnt); | ||
| FT_ATOMIC_ADD_SSIZE(soself->mutex_cnt, 1); | ||
| if ( s_pack_internal(soself, args, 0, buf, state) != 0 ){ | ||
| FT_ATOMIC_STORE_SSIZE(soself->mutex_cnt, prev_cnt); | ||
| PyBytesWriter_Discard(writer); | ||
| return NULL; | ||
| } | ||
| FT_ATOMIC_STORE_SSIZE(soself->mutex_cnt, prev_cnt); | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it work to use And then add maybe | ||
| return PyBytesWriter_FinishWithSize(writer, soself->s_size); | ||
| } | ||
| @@ -2360,10 +2372,15 @@ s_pack_into(PyObject *self, PyObject *const *args, Py_ssize_t nargs) | ||
| } | ||
| /* Call the guts */ | ||
| Py_ssize_t prev_cnt = FT_ATOMIC_LOAD_SSIZE(soself->mutex_cnt); | ||
| FT_ATOMIC_ADD_SSIZE(soself->mutex_cnt, 1); | ||
| if (s_pack_internal(soself, args, 2, (char*)buffer.buf + offset, state) != 0){ | ||
| FT_ATOMIC_STORE_SSIZE(soself->mutex_cnt, prev_cnt); | ||
| PyBuffer_Release(&buffer); | ||
| return NULL; | ||
| } | ||
| FT_ATOMIC_STORE_SSIZE(soself->mutex_cnt, prev_cnt); | ||
| PyBuffer_Release(&buffer); | ||
| Py_RETURN_NONE; | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.