Commit 8d53d2bc authored by Julian Andres Klode's avatar Julian Andres Klode
Browse files

File descriptor leaks in ArArchive, DebFile, TagFile

Fix various file descritor, and memory leaks in ArArchive, DebFile,
and TagFile by introducing a new PyApt_UniqueObject smart pointer
that is like a unique_ptr, but backportable to older releases, and
automatically clears subobjects, so objects with cycles like DebFile
and TagFile will be released on error paths.

LP: #1899193
GHSL-2020-170
CVE-2020-27351
parent a228b580
......@@ -389,22 +389,22 @@ static PyObject *ararchive_new(PyTypeObject *type, PyObject *args,
PyObject *kwds)
{
PyObject *file;
PyArArchiveObject *self;
PyApt_Filename filename;
int fileno;
if (PyArg_ParseTuple(args,"O:__new__",&file) == 0)
return 0;
PyApt_UniqueObject<PyArArchiveObject> self(NULL);
// We receive a filename.
if (filename.init(file)) {
self = (PyArArchiveObject *)CppPyObject_NEW<ARArchive*>(0,type);
self.reset((PyArArchiveObject*) CppPyObject_NEW<ARArchive*>(0,type));
new (&self->Fd) FileFd(filename,FileFd::ReadOnly);
}
// We receive a file object.
else if ((fileno = PyObject_AsFileDescriptor(file)) != -1) {
// Clear the error set by PyObject_AsString().
PyErr_Clear();
self = (PyArArchiveObject *)CppPyObject_NEW<ARArchive*>(file,type);
self.reset((PyArArchiveObject*) CppPyObject_NEW<ARArchive*>(file,type));
new (&self->Fd) FileFd(fileno,false);
}
else {
......@@ -413,7 +413,7 @@ static PyObject *ararchive_new(PyTypeObject *type, PyObject *args,
self->Object = (PyARArchiveHack*)new ARArchive(self->Fd);
if (_error->PendingError() == true)
return HandleErrors();
return self;
return self.release();
}
static void ararchive_dealloc(PyObject *self)
......@@ -578,16 +578,16 @@ static PyObject *debfile_get_tar(PyDebFileObject *self, const char *Name)
static PyObject *debfile_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
PyDebFileObject *self = (PyDebFileObject*)ararchive_new(type, args, kwds);
PyApt_UniqueObject<PyDebFileObject> self((PyDebFileObject*)ararchive_new(type, args, kwds));
if (self == NULL)
return NULL;
// DebFile
self->control = debfile_get_tar(self, "control.tar");
self->control = debfile_get_tar(self.get(), "control.tar");
if (self->control == NULL)
return NULL;
self->data = debfile_get_tar(self, "data.tar");
self->data = debfile_get_tar(self.get(), "data.tar");
if (self->data == NULL)
return NULL;
......@@ -603,7 +603,7 @@ static PyObject *debfile_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
self->Fd.Read(value, member->Size, true);
self->debian_binary = PyBytes_FromStringAndSize(value, member->Size);
delete[] value;
return self;
return self.release();
}
static int debfile_traverse(PyObject *_self, visitproc visit, void* arg)
......
......@@ -309,4 +309,23 @@ public:
}
};
/**
* Basic smart pointer to hold initial objects.
*
* This is like a std::unique_ptr<PyObject, decltype(&Py_DecRef)> to some extend,
* but it is for initialization only, and hence will also clear out any members
* in case it deletes the instance (the error case).
*/
template <class T, bool clear=true> struct PyApt_UniqueObject {
T *self;
explicit PyApt_UniqueObject(T *self) : self(self) { }
~PyApt_UniqueObject() { reset(NULL); }
void reset(T *newself) { if (clear && self && Py_TYPE(self)->tp_clear) Py_TYPE(self)->tp_clear(self); Py_XDECREF(self); self = newself; }
PyApt_UniqueObject<T> operator =(PyApt_UniqueObject<T>) = delete;
bool operator ==(void *other) { return self == other; }
T *operator ->() { return self; }
T *get() { return self; }
T *release() { T *ret = self; self = NULL; return ret; }
};
#endif
......@@ -534,7 +534,6 @@ static PyObject *TagSecNew(PyTypeObject *type,PyObject *Args,PyObject *kwds) {
static PyObject *TagFileNew(PyTypeObject *type,PyObject *Args,PyObject *kwds)
{
TagFileData *New;
PyObject *File = 0;
char Bytes = 0;
......@@ -558,7 +557,7 @@ static PyObject *TagFileNew(PyTypeObject *type,PyObject *Args,PyObject *kwds)
return 0;
}
New = (TagFileData*)type->tp_alloc(type, 0);
PyApt_UniqueObject<TagFileData> New((TagFileData*)type->tp_alloc(type, 0));
if (fileno != -1)
{
#ifdef APT_HAS_GZIP
......@@ -596,7 +595,7 @@ static PyObject *TagFileNew(PyTypeObject *type,PyObject *Args,PyObject *kwds)
// Create the section
New->Section = (TagSecData*)(&PyTagSection_Type)->tp_alloc(&PyTagSection_Type, 0);
new (&New->Section->Object) pkgTagSection();
New->Section->Owner = New;
New->Section->Owner = New.get();
Py_INCREF(New->Section->Owner);
New->Section->Data = 0;
New->Section->Bytes = Bytes;
......@@ -605,7 +604,7 @@ static PyObject *TagFileNew(PyTypeObject *type,PyObject *Args,PyObject *kwds)
Py_XINCREF(New->Section->Encoding);
#endif
return HandleErrors(New);
return HandleErrors(New.release());
}
/*}}}*/
......
......@@ -341,7 +341,6 @@ PyTypeObject PyTarMember_Type = {
static PyObject *tarfile_new(PyTypeObject *type,PyObject *args,PyObject *kwds)
{
PyObject *file;
PyTarFileObject *self;
PyApt_Filename filename;
int fileno;
int min = 0;
......@@ -353,7 +352,7 @@ static PyObject *tarfile_new(PyTypeObject *type,PyObject *args,PyObject *kwds)
&max,&comp) == 0)
return 0;
self = (PyTarFileObject*)CppPyObject_NEW<ExtractTar*>(file,type);
PyApt_UniqueObject<PyTarFileObject> self((PyTarFileObject*)CppPyObject_NEW<ExtractTar*>(file,type));
// We receive a filename.
if (filename.init(file))
......@@ -364,15 +363,14 @@ static PyObject *tarfile_new(PyTypeObject *type,PyObject *args,PyObject *kwds)
new (&self->Fd) FileFd(fileno,false);
}
else {
Py_DECREF(self);
return 0;
}
self->min = min;
self->Object = new ExtractTar(self->Fd,max,comp);
if (_error->PendingError() == true)
return HandleErrors(self);
return self;
return HandleErrors(self.release());
return self.release();
}
static const char *tarfile_extractall_doc =
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment