From d65b7641c84976c543ded8f0de5ab2da3c19b407 Mon Sep 17 00:00:00 2001 From: Gary Lockyer Date: Wed, 8 May 2019 11:30:20 +1200 Subject: [PATCH] s4 librpc rpc pyrpc: Ensure tevent_context deleted last Ensure that the tevent_context is deleted after the connection, to prevent a use after free. Note: Py_DECREF calls dcerpc_interface_dealloc so the TALLOC_FREE(ret->mem_ctx) calls in the error paths of py_dcerpc_interface_init_helper needed removal. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13932 Signed-off-by: Gary Lockyer Reviewed-by: Andrew Bartlett --- source4/librpc/rpc/pyrpc.c | 18 +++++++++++++ source4/librpc/rpc/pyrpc.h | 1 + source4/librpc/rpc/pyrpc_util.c | 48 ++++++++++++++------------------- 3 files changed, 39 insertions(+), 28 deletions(-) diff --git a/source4/librpc/rpc/pyrpc.c b/source4/librpc/rpc/pyrpc.c index e00318a96be..0713920e3a5 100644 --- a/source4/librpc/rpc/pyrpc.c +++ b/source4/librpc/rpc/pyrpc.c @@ -300,9 +300,27 @@ static PyMethodDef dcerpc_interface_methods[] = { static void dcerpc_interface_dealloc(PyObject* self) { dcerpc_InterfaceObject *interface = (dcerpc_InterfaceObject *)self; + + /* + * This can't fail, and if it did talloc_unlink(NULL, NULL) is + * harmless below + */ + struct tevent_context *ev_save = talloc_reparent( + NULL, interface->mem_ctx, interface->ev); + interface->binding_handle = NULL; interface->pipe = NULL; + + /* + * Free everything *except* the event context, which must go + * away last + */ TALLOC_FREE(interface->mem_ctx); + + /* + * Now wish a fond goodbye to the event context itself + */ + talloc_unlink(NULL, ev_save); self->ob_type->tp_free(self); } diff --git a/source4/librpc/rpc/pyrpc.h b/source4/librpc/rpc/pyrpc.h index 7101e7345de..311ba2d294d 100644 --- a/source4/librpc/rpc/pyrpc.h +++ b/source4/librpc/rpc/pyrpc.h @@ -49,6 +49,7 @@ typedef struct { TALLOC_CTX *mem_ctx; struct dcerpc_pipe *pipe; struct dcerpc_binding_handle *binding_handle; + struct tevent_context *ev; } dcerpc_InterfaceObject; diff --git a/source4/librpc/rpc/pyrpc_util.c b/source4/librpc/rpc/pyrpc_util.c index 8d5ec45817f..8015e709964 100644 --- a/source4/librpc/rpc/pyrpc_util.c +++ b/source4/librpc/rpc/pyrpc_util.c @@ -123,6 +123,7 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py ret->pipe = NULL; ret->binding_handle = NULL; + ret->ev = NULL; ret->mem_ctx = talloc_new(NULL); if (ret->mem_ctx == NULL) { PyErr_NoMemory(); @@ -130,30 +131,26 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py } if (strncmp(binding_string, "irpc:", 5) == 0) { - struct tevent_context *event_ctx; struct loadparm_context *lp_ctx; - event_ctx = s4_event_context_init(ret->mem_ctx); - if (event_ctx == NULL) { + ret->ev = s4_event_context_init(ret->mem_ctx); + if (ret->ev == NULL) { PyErr_SetString(PyExc_TypeError, "Expected loadparm context"); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); return NULL; } - lp_ctx = lpcfg_from_py_object(event_ctx, py_lp_ctx); + lp_ctx = lpcfg_from_py_object(ret->ev, py_lp_ctx); if (lp_ctx == NULL) { PyErr_SetString(PyExc_TypeError, "Expected loadparm context"); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); return NULL; } status = pyrpc_irpc_connect(ret->mem_ctx, binding_string+5, table, - event_ctx, lp_ctx, &ret->binding_handle); + ret->ev, lp_ctx, &ret->binding_handle); if (!NT_STATUS_IS_OK(status)) { PyErr_SetNTSTATUS(status); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); return NULL; } @@ -164,7 +161,6 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py py_base = PyImport_ImportModule("samba.dcerpc.base"); if (py_base == NULL) { - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); return NULL; } @@ -172,7 +168,6 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py ClientConnection_Type = (PyTypeObject *)PyObject_GetAttrString(py_base, "ClientConnection"); if (ClientConnection_Type == NULL) { PyErr_SetNone(PyExc_TypeError); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); Py_DECREF(py_base); return NULL; @@ -180,7 +175,6 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py if (!PyObject_TypeCheck(py_basis, ClientConnection_Type)) { PyErr_SetString(PyExc_TypeError, "basis_connection must be a DCE/RPC connection"); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); Py_DECREF(py_base); Py_DECREF(ClientConnection_Type); @@ -191,7 +185,17 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py ((dcerpc_InterfaceObject *)py_basis)->pipe); if (base_pipe == NULL) { PyErr_NoMemory(); - TALLOC_FREE(ret->mem_ctx); + Py_DECREF(ret); + Py_DECREF(py_base); + Py_DECREF(ClientConnection_Type); + return NULL; + } + + ret->ev = talloc_reference( + ret->mem_ctx, + ((dcerpc_InterfaceObject *)py_basis)->ev); + if (ret->ev == NULL) { + PyErr_NoMemory(); Py_DECREF(ret); Py_DECREF(py_base); Py_DECREF(ClientConnection_Type); @@ -201,7 +205,6 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py status = dcerpc_secondary_context(base_pipe, &ret->pipe, table); if (!NT_STATUS_IS_OK(status)) { PyErr_SetNTSTATUS(status); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); Py_DECREF(py_base); Py_DECREF(ClientConnection_Type); @@ -212,22 +215,19 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py Py_XDECREF(ClientConnection_Type); Py_XDECREF(py_base); } else { - struct tevent_context *event_ctx; struct loadparm_context *lp_ctx; struct cli_credentials *credentials; - event_ctx = s4_event_context_init(ret->mem_ctx); - if (event_ctx == NULL) { + ret->ev = s4_event_context_init(ret->mem_ctx); + if (ret->ev == NULL) { PyErr_SetString(PyExc_TypeError, "Expected loadparm context"); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); return NULL; } - lp_ctx = lpcfg_from_py_object(event_ctx, py_lp_ctx); + lp_ctx = lpcfg_from_py_object(ret->ev, py_lp_ctx); if (lp_ctx == NULL) { PyErr_SetString(PyExc_TypeError, "Expected loadparm context"); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); return NULL; } @@ -235,24 +235,16 @@ PyObject *py_dcerpc_interface_init_helper(PyTypeObject *type, PyObject *args, Py credentials = cli_credentials_from_py_object(py_credentials); if (credentials == NULL) { PyErr_SetString(PyExc_TypeError, "Expected credentials"); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); return NULL; } status = dcerpc_pipe_connect(ret->mem_ctx, &ret->pipe, binding_string, - table, credentials, event_ctx, lp_ctx); + table, credentials, ret->ev, lp_ctx); if (!NT_STATUS_IS_OK(status)) { PyErr_SetNTSTATUS(status); - TALLOC_FREE(ret->mem_ctx); Py_DECREF(ret); return NULL; } - - /* - * the event context is cached under the connection, - * so let it be a child of it. - */ - talloc_steal(ret->pipe->conn, event_ctx); } if (ret->pipe) { -- 2.34.1