From 5152303c4a5528a71aeafffcca43e7df4aad785c Mon Sep 17 00:00:00 2001 From: Zackery Spytz Date: Wed, 20 Jun 2018 09:12:39 -0600 Subject: [PATCH] bpo-23224: Fix segfaults and multiple leaks in the lzma and bz2 modules lzma.LZMADecompressor and bz2.BZ2Decompressor objects caused segfaults when their __init__() methods were not called. lzma.LZMADecompressor, lzma.LZMACompressor, bz2.BZ2Compressor, and bz2.BZ2Decompressor objects would leak locks and internal buffers when their __init__() methods were called multiple times. --- Lib/test/test_bz2.py | 4 + Lib/test/test_lzma.py | 4 + .../2018-06-20-09-12-21.bpo-23224.zxCQ13.rst | 6 + Modules/_bz2module.c | 69 +++++---- Modules/_lzmamodule.c | 140 +++++++++++------- Modules/clinic/_bz2module.c.h | 36 ++--- Modules/clinic/_lzmamodule.c.h | 18 +-- 7 files changed, 167 insertions(+), 110 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-06-20-09-12-21.bpo-23224.zxCQ13.rst diff --git a/Lib/test/test_bz2.py b/Lib/test/test_bz2.py index 003497f28b16244..b522c58225cc3e6 100644 --- a/Lib/test/test_bz2.py +++ b/Lib/test/test_bz2.py @@ -826,6 +826,10 @@ def test_refleaks_in___init__(self): bzd.__init__() self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + def test_uninitialized_BZ2Decompressor_crash(self): + self.assertEqual(BZ2Decompressor.__new__(BZ2Decompressor). + decompress(bytes()), b'') + class CompressDecompressTest(BaseTest): def testCompress(self): diff --git a/Lib/test/test_lzma.py b/Lib/test/test_lzma.py index 3dc2c1e7e3b779e..5886b3c39d70b78 100644 --- a/Lib/test/test_lzma.py +++ b/Lib/test/test_lzma.py @@ -375,6 +375,10 @@ def test_refleaks_in_decompressor___init__(self): lzd.__init__() self.assertAlmostEqual(gettotalrefcount() - refs_before, 0, delta=10) + def test_uninitialized_LZMADecompressor_crash(self): + self.assertEqual(LZMADecompressor.__new__(LZMADecompressor). + decompress(bytes()), b'') + class CompressDecompressFunctionTestCase(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2018-06-20-09-12-21.bpo-23224.zxCQ13.rst b/Misc/NEWS.d/next/Library/2018-06-20-09-12-21.bpo-23224.zxCQ13.rst new file mode 100644 index 000000000000000..8909753c7f9ee62 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-06-20-09-12-21.bpo-23224.zxCQ13.rst @@ -0,0 +1,6 @@ +Fix segfaults when creating :class:`lzma.LZMADecompressor` and +:class:`bz2.BZ2Decompressor` objects without calling ``__init__()``, and fix +leakage of locks and internal buffers when calling the ``__init__()`` +methods of :class:`lzma.LZMADecompressor`, :class:`lzma.LZMACompressor`, +:class:`bz2.BZ2Compressor`, and :class:`bz2.BZ2Decompressor` objects +multiple times. diff --git a/Modules/_bz2module.c b/Modules/_bz2module.c index 0789b6179e52be7..d8543e52930b244 100644 --- a/Modules/_bz2module.c +++ b/Modules/_bz2module.c @@ -291,7 +291,8 @@ BZ2_Free(void* ctx, void *ptr) } /*[clinic input] -_bz2.BZ2Compressor.__init__ +@classmethod +_bz2.BZ2Compressor.__new__ compresslevel: int = 9 Compression level, as a number between 1 and 9. @@ -302,22 +303,30 @@ Create a compressor object for compressing data incrementally. For one-shot compression, use the compress() function instead. [clinic start generated code]*/ -static int -_bz2_BZ2Compressor___init___impl(BZ2Compressor *self, int compresslevel) -/*[clinic end generated code: output=c4e6adfd02963827 input=4e1ff7b8394b6e9a]*/ +static PyObject * +_bz2_BZ2Compressor_impl(PyTypeObject *type, int compresslevel) +/*[clinic end generated code: output=83346c96beaacad7 input=d4500d2a52c8b263]*/ { int bzerror; + BZ2Compressor *self; if (!(1 <= compresslevel && compresslevel <= 9)) { PyErr_SetString(PyExc_ValueError, "compresslevel must be between 1 and 9"); - return -1; + return NULL; + } + + assert(type != NULL && type->tp_alloc != NULL); + self = (BZ2Compressor *)type->tp_alloc(type, 0); + if (self == NULL) { + return NULL; } self->lock = PyThread_allocate_lock(); if (self->lock == NULL) { + Py_DECREF(self); PyErr_SetString(PyExc_MemoryError, "Unable to allocate lock"); - return -1; + return NULL; } self->bzs.opaque = NULL; @@ -327,12 +336,11 @@ _bz2_BZ2Compressor___init___impl(BZ2Compressor *self, int compresslevel) if (catch_bz2_error(bzerror)) goto error; - return 0; + return (PyObject *)self; error: - PyThread_free_lock(self->lock); - self->lock = NULL; - return -1; + Py_DECREF(self); + return NULL; } static void @@ -373,7 +381,7 @@ static PyTypeObject BZ2Compressor_Type = { 0, /* tp_setattro */ 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT, /* tp_flags */ - _bz2_BZ2Compressor___init____doc__, /* tp_doc */ + _bz2_BZ2Compressor__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ 0, /* tp_richcompare */ @@ -388,9 +396,9 @@ static PyTypeObject BZ2Compressor_Type = { 0, /* tp_descr_get */ 0, /* tp_descr_set */ 0, /* tp_dictoffset */ - _bz2_BZ2Compressor___init__, /* tp_init */ + 0, /* tp_init */ 0, /* tp_alloc */ - PyType_GenericNew, /* tp_new */ + _bz2_BZ2Compressor, /* tp_new */ }; @@ -621,30 +629,39 @@ BZ2Decompressor_getstate(BZ2Decompressor *self, PyObject *noargs) } /*[clinic input] -_bz2.BZ2Decompressor.__init__ +@classmethod +_bz2.BZ2Decompressor.__new__ Create a decompressor object for decompressing data incrementally. For one-shot decompression, use the decompress() function instead. [clinic start generated code]*/ -static int -_bz2_BZ2Decompressor___init___impl(BZ2Decompressor *self) -/*[clinic end generated code: output=e4d2b9bb866ab8f1 input=95f6500dcda60088]*/ +static PyObject * +_bz2_BZ2Decompressor_impl(PyTypeObject *type) +/*[clinic end generated code: output=5150d51ccaab220e input=b87413ce51853528]*/ { + BZ2Decompressor *self; int bzerror; + assert(type != NULL && type->tp_alloc != NULL); + self = (BZ2Decompressor *)type->tp_alloc(type, 0); + if (self == NULL) { + return NULL; + } + self->lock = PyThread_allocate_lock(); if (self->lock == NULL) { + Py_DECREF(self); PyErr_SetString(PyExc_MemoryError, "Unable to allocate lock"); - return -1; + return NULL; } self->needs_input = 1; self->bzs_avail_in_real = 0; self->input_buffer = NULL; self->input_buffer_size = 0; - Py_XSETREF(self->unused_data, PyBytes_FromStringAndSize(NULL, 0)); + self->unused_data = PyBytes_FromStringAndSize(NULL, 0); if (self->unused_data == NULL) goto error; @@ -652,13 +669,11 @@ _bz2_BZ2Decompressor___init___impl(BZ2Decompressor *self) if (catch_bz2_error(bzerror)) goto error; - return 0; + return (PyObject *)self; error: - Py_CLEAR(self->unused_data); - PyThread_free_lock(self->lock); - self->lock = NULL; - return -1; + Py_DECREF(self); + return NULL; } static void @@ -719,7 +734,7 @@ static PyTypeObject BZ2Decompressor_Type = { 0, /* tp_setattro */ 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT, /* tp_flags */ - _bz2_BZ2Decompressor___init____doc__, /* tp_doc */ + _bz2_BZ2Decompressor__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ 0, /* tp_richcompare */ @@ -734,9 +749,9 @@ static PyTypeObject BZ2Decompressor_Type = { 0, /* tp_descr_get */ 0, /* tp_descr_set */ 0, /* tp_dictoffset */ - _bz2_BZ2Decompressor___init__, /* tp_init */ + 0, /* tp_init */ 0, /* tp_alloc */ - PyType_GenericNew, /* tp_new */ + _bz2_BZ2Decompressor, /* tp_new */ }; diff --git a/Modules/_lzmamodule.c b/Modules/_lzmamodule.c index 5bcd088d7721883..b9b045e5dba64de 100644 --- a/Modules/_lzmamodule.c +++ b/Modules/_lzmamodule.c @@ -709,8 +709,8 @@ the raw compressor does not support preset compression levels. For one-shot compression, use the compress() function instead. [-clinic start generated code]*/ -static int -Compressor_init(Compressor *self, PyObject *args, PyObject *kwargs) +static PyObject * +Compressor_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) { static char *arg_names[] = {"format", "check", "preset", "filters", NULL}; int format = FORMAT_XZ; @@ -718,28 +718,37 @@ Compressor_init(Compressor *self, PyObject *args, PyObject *kwargs) uint32_t preset = LZMA_PRESET_DEFAULT; PyObject *preset_obj = Py_None; PyObject *filterspecs = Py_None; + Compressor *self; if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|iiOO:LZMACompressor", arg_names, &format, &check, &preset_obj, &filterspecs)) - return -1; + { + return NULL; + } if (format != FORMAT_XZ && check != -1 && check != LZMA_CHECK_NONE) { PyErr_SetString(PyExc_ValueError, "Integrity checks are only supported by FORMAT_XZ"); - return -1; + return NULL; } if (preset_obj != Py_None && filterspecs != Py_None) { PyErr_SetString(PyExc_ValueError, "Cannot specify both preset and filter chain"); - return -1; + return NULL; } - if (preset_obj != Py_None) - if (!uint32_converter(preset_obj, &preset)) - return -1; + if (preset_obj != Py_None && !uint32_converter(preset_obj, &preset)) { + return NULL; + } + + assert(type != NULL && type->tp_alloc != NULL); + self = (Compressor *)type->tp_alloc(type, 0); + if (self == NULL) { + return NULL; + } self->alloc.opaque = NULL; self->alloc.alloc = PyLzma_Malloc; @@ -748,8 +757,9 @@ Compressor_init(Compressor *self, PyObject *args, PyObject *kwargs) self->lock = PyThread_allocate_lock(); if (self->lock == NULL) { + Py_DECREF(self); PyErr_SetString(PyExc_MemoryError, "Unable to allocate lock"); - return -1; + return NULL; } self->flushed = 0; @@ -757,29 +767,34 @@ Compressor_init(Compressor *self, PyObject *args, PyObject *kwargs) case FORMAT_XZ: if (check == -1) check = LZMA_CHECK_CRC64; - if (Compressor_init_xz(&self->lzs, check, preset, filterspecs) != 0) - break; - return 0; + if (Compressor_init_xz(&self->lzs, check, preset, filterspecs) != 0) { + goto error; + } + break; case FORMAT_ALONE: - if (Compressor_init_alone(&self->lzs, preset, filterspecs) != 0) - break; - return 0; + if (Compressor_init_alone(&self->lzs, preset, filterspecs) != 0) { + goto error; + } + break; case FORMAT_RAW: - if (Compressor_init_raw(&self->lzs, filterspecs) != 0) - break; - return 0; + if (Compressor_init_raw(&self->lzs, filterspecs) != 0) { + goto error; + } + break; default: PyErr_Format(PyExc_ValueError, "Invalid container format: %d", format); - break; + goto error; } - PyThread_free_lock(self->lock); - self->lock = NULL; - return -1; + return (PyObject *)self; + +error: + Py_DECREF(self); + return NULL; } static void @@ -862,9 +877,9 @@ static PyTypeObject Compressor_type = { 0, /* tp_descr_get */ 0, /* tp_descr_set */ 0, /* tp_dictoffset */ - (initproc)Compressor_init, /* tp_init */ + 0, /* tp_init */ 0, /* tp_alloc */ - PyType_GenericNew, /* tp_new */ + Compressor_new, /* tp_new */ }; @@ -1103,7 +1118,8 @@ Decompressor_init_raw(lzma_stream *lzs, PyObject *filterspecs) } /*[clinic input] -_lzma.LZMADecompressor.__init__ +@classmethod +_lzma.LZMADecompressor.__new__ format: int(c_default="FORMAT_AUTO") = FORMAT_AUTO Specifies the container format of the input stream. If this is @@ -1127,11 +1143,12 @@ Create a decompressor object for decompressing data incrementally. For one-shot decompression, use the decompress() function instead. [clinic start generated code]*/ -static int -_lzma_LZMADecompressor___init___impl(Decompressor *self, int format, - PyObject *memlimit, PyObject *filters) -/*[clinic end generated code: output=3e1821f8aa36564c input=81fe684a6c2f8a27]*/ +static PyObject * +_lzma_LZMADecompressor_impl(PyTypeObject *type, int format, + PyObject *memlimit, PyObject *filters) +/*[clinic end generated code: output=2d46d5e70f10bc7f input=ca40cd1cb1202b0d]*/ { + Decompressor *self; const uint32_t decoder_flags = LZMA_TELL_ANY_CHECK | LZMA_TELL_NO_CHECK; uint64_t memlimit_ = UINT64_MAX; lzma_ret lzret; @@ -1140,23 +1157,29 @@ _lzma_LZMADecompressor___init___impl(Decompressor *self, int format, if (format == FORMAT_RAW) { PyErr_SetString(PyExc_ValueError, "Cannot specify memory limit with FORMAT_RAW"); - return -1; + return NULL; } memlimit_ = PyLong_AsUnsignedLongLong(memlimit); - if (PyErr_Occurred()) - return -1; + if (PyErr_Occurred()) { + return NULL; + } } if (format == FORMAT_RAW && filters == Py_None) { PyErr_SetString(PyExc_ValueError, "Must specify filters for FORMAT_RAW"); - return -1; + return NULL; } else if (format != FORMAT_RAW && filters != Py_None) { PyErr_SetString(PyExc_ValueError, "Cannot specify filters except with FORMAT_RAW"); - return -1; + return NULL; } + assert(type != NULL && type->tp_alloc != NULL); + self = (Decompressor *)type->tp_alloc(type, 0); + if (self == NULL) { + return NULL; + } self->alloc.opaque = NULL; self->alloc.alloc = PyLzma_Malloc; self->alloc.free = PyLzma_Free; @@ -1165,55 +1188,60 @@ _lzma_LZMADecompressor___init___impl(Decompressor *self, int format, self->lock = PyThread_allocate_lock(); if (self->lock == NULL) { + Py_DECREF(self); PyErr_SetString(PyExc_MemoryError, "Unable to allocate lock"); - return -1; + return NULL; } self->check = LZMA_CHECK_UNKNOWN; self->needs_input = 1; self->input_buffer = NULL; self->input_buffer_size = 0; - Py_XSETREF(self->unused_data, PyBytes_FromStringAndSize(NULL, 0)); + self->unused_data = PyBytes_FromStringAndSize(NULL, 0); if (self->unused_data == NULL) goto error; switch (format) { case FORMAT_AUTO: lzret = lzma_auto_decoder(&self->lzs, memlimit_, decoder_flags); - if (catch_lzma_error(lzret)) - break; - return 0; + if (catch_lzma_error(lzret)) { + goto error; + } + break; case FORMAT_XZ: lzret = lzma_stream_decoder(&self->lzs, memlimit_, decoder_flags); - if (catch_lzma_error(lzret)) - break; - return 0; + if (catch_lzma_error(lzret)) { + goto error; + } + break; case FORMAT_ALONE: self->check = LZMA_CHECK_NONE; lzret = lzma_alone_decoder(&self->lzs, memlimit_); - if (catch_lzma_error(lzret)) - break; - return 0; + if (catch_lzma_error(lzret)) { + goto error; + } + break; case FORMAT_RAW: self->check = LZMA_CHECK_NONE; - if (Decompressor_init_raw(&self->lzs, filters) == -1) - break; - return 0; + if (Decompressor_init_raw(&self->lzs, filters) == -1) { + goto error; + } + break; default: PyErr_Format(PyExc_ValueError, "Invalid container format: %d", format); - break; + goto error; } + return (PyObject *)self; + error: - Py_CLEAR(self->unused_data); - PyThread_free_lock(self->lock); - self->lock = NULL; - return -1; + Py_DECREF(self); + return NULL; } static void @@ -1280,7 +1308,7 @@ static PyTypeObject Decompressor_type = { 0, /* tp_setattro */ 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT, /* tp_flags */ - _lzma_LZMADecompressor___init____doc__, /* tp_doc */ + _lzma_LZMADecompressor__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ 0, /* tp_richcompare */ @@ -1295,9 +1323,9 @@ static PyTypeObject Decompressor_type = { 0, /* tp_descr_get */ 0, /* tp_descr_set */ 0, /* tp_dictoffset */ - _lzma_LZMADecompressor___init__, /* tp_init */ + 0, /* tp_init */ 0, /* tp_alloc */ - PyType_GenericNew, /* tp_new */ + _lzma_LZMADecompressor, /* tp_new */ }; diff --git a/Modules/clinic/_bz2module.c.h b/Modules/clinic/_bz2module.c.h index 601bcc690d95585..beda26ce90eafd9 100644 --- a/Modules/clinic/_bz2module.c.h +++ b/Modules/clinic/_bz2module.c.h @@ -61,7 +61,7 @@ _bz2_BZ2Compressor_flush(BZ2Compressor *self, PyObject *Py_UNUSED(ignored)) return _bz2_BZ2Compressor_flush_impl(self); } -PyDoc_STRVAR(_bz2_BZ2Compressor___init____doc__, +PyDoc_STRVAR(_bz2_BZ2Compressor__doc__, "BZ2Compressor(compresslevel=9, /)\n" "--\n" "\n" @@ -72,16 +72,16 @@ PyDoc_STRVAR(_bz2_BZ2Compressor___init____doc__, "\n" "For one-shot compression, use the compress() function instead."); -static int -_bz2_BZ2Compressor___init___impl(BZ2Compressor *self, int compresslevel); +static PyObject * +_bz2_BZ2Compressor_impl(PyTypeObject *type, int compresslevel); -static int -_bz2_BZ2Compressor___init__(PyObject *self, PyObject *args, PyObject *kwargs) +static PyObject * +_bz2_BZ2Compressor(PyTypeObject *type, PyObject *args, PyObject *kwargs) { - int return_value = -1; + PyObject *return_value = NULL; int compresslevel = 9; - if ((Py_TYPE(self) == &BZ2Compressor_Type) && + if ((type == &BZ2Compressor_Type) && !_PyArg_NoKeywords("BZ2Compressor", kwargs)) { goto exit; } @@ -89,7 +89,7 @@ _bz2_BZ2Compressor___init__(PyObject *self, PyObject *args, PyObject *kwargs) &compresslevel)) { goto exit; } - return_value = _bz2_BZ2Compressor___init___impl((BZ2Compressor *)self, compresslevel); + return_value = _bz2_BZ2Compressor_impl(type, compresslevel); exit: return return_value; @@ -145,7 +145,7 @@ _bz2_BZ2Decompressor_decompress(BZ2Decompressor *self, PyObject *const *args, Py return return_value; } -PyDoc_STRVAR(_bz2_BZ2Decompressor___init____doc__, +PyDoc_STRVAR(_bz2_BZ2Decompressor__doc__, "BZ2Decompressor()\n" "--\n" "\n" @@ -153,25 +153,25 @@ PyDoc_STRVAR(_bz2_BZ2Decompressor___init____doc__, "\n" "For one-shot decompression, use the decompress() function instead."); -static int -_bz2_BZ2Decompressor___init___impl(BZ2Decompressor *self); +static PyObject * +_bz2_BZ2Decompressor_impl(PyTypeObject *type); -static int -_bz2_BZ2Decompressor___init__(PyObject *self, PyObject *args, PyObject *kwargs) +static PyObject * +_bz2_BZ2Decompressor(PyTypeObject *type, PyObject *args, PyObject *kwargs) { - int return_value = -1; + PyObject *return_value = NULL; - if ((Py_TYPE(self) == &BZ2Decompressor_Type) && + if ((type == &BZ2Decompressor_Type) && !_PyArg_NoPositional("BZ2Decompressor", args)) { goto exit; } - if ((Py_TYPE(self) == &BZ2Decompressor_Type) && + if ((type == &BZ2Decompressor_Type) && !_PyArg_NoKeywords("BZ2Decompressor", kwargs)) { goto exit; } - return_value = _bz2_BZ2Decompressor___init___impl((BZ2Decompressor *)self); + return_value = _bz2_BZ2Decompressor_impl(type); exit: return return_value; } -/*[clinic end generated code: output=564a0313177fff63 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=558c3b1efdd19d66 input=a9049054013a1b77]*/ diff --git a/Modules/clinic/_lzmamodule.c.h b/Modules/clinic/_lzmamodule.c.h index ed7eb5df0522cc2..9f5f246cd7705ff 100644 --- a/Modules/clinic/_lzmamodule.c.h +++ b/Modules/clinic/_lzmamodule.c.h @@ -111,7 +111,7 @@ _lzma_LZMADecompressor_decompress(Decompressor *self, PyObject *const *args, Py_ return return_value; } -PyDoc_STRVAR(_lzma_LZMADecompressor___init____doc__, +PyDoc_STRVAR(_lzma_LZMADecompressor__doc__, "LZMADecompressor(format=FORMAT_AUTO, memlimit=None, filters=None)\n" "--\n" "\n" @@ -134,14 +134,14 @@ PyDoc_STRVAR(_lzma_LZMADecompressor___init____doc__, "\n" "For one-shot decompression, use the decompress() function instead."); -static int -_lzma_LZMADecompressor___init___impl(Decompressor *self, int format, - PyObject *memlimit, PyObject *filters); +static PyObject * +_lzma_LZMADecompressor_impl(PyTypeObject *type, int format, + PyObject *memlimit, PyObject *filters); -static int -_lzma_LZMADecompressor___init__(PyObject *self, PyObject *args, PyObject *kwargs) +static PyObject * +_lzma_LZMADecompressor(PyTypeObject *type, PyObject *args, PyObject *kwargs) { - int return_value = -1; + PyObject *return_value = NULL; static const char * const _keywords[] = {"format", "memlimit", "filters", NULL}; static _PyArg_Parser _parser = {"|iOO:LZMADecompressor", _keywords, 0}; int format = FORMAT_AUTO; @@ -152,7 +152,7 @@ _lzma_LZMADecompressor___init__(PyObject *self, PyObject *args, PyObject *kwargs &format, &memlimit, &filters)) { goto exit; } - return_value = _lzma_LZMADecompressor___init___impl((Decompressor *)self, format, memlimit, filters); + return_value = _lzma_LZMADecompressor_impl(type, format, memlimit, filters); exit: return return_value; @@ -256,4 +256,4 @@ _lzma__decode_filter_properties(PyObject *module, PyObject *const *args, Py_ssiz return return_value; } -/*[clinic end generated code: output=38c2d52362bf3712 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=4df2398e6a198f8e input=a9049054013a1b77]*/