From 784ee21616a60993ecc0979f83fbb04467d57af9 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Wed, 8 Nov 2023 10:43:38 +1300 Subject: [PATCH] pyldb: Include a reference to the Ldb in objects that use This will help avoid use-after-free of the internally cached ldb within struct ldb_dn by ensuring that it lives as long. Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall --- lib/ldb/ABI/pyldb-util-2.9.0.sigs | 2 +- lib/ldb/pyldb.c | 86 ++++++++++++++++++------- lib/ldb/pyldb.h | 14 +++- lib/ldb/pyldb_util.c | 5 +- selftest/knownfail.d/ldb-use-after-free | 11 ---- source4/dns_server/pydns.c | 2 +- source4/dsdb/pydsdb.c | 9 +-- 7 files changed, 85 insertions(+), 44 deletions(-) delete mode 100644 selftest/knownfail.d/ldb-use-after-free diff --git a/lib/ldb/ABI/pyldb-util-2.9.0.sigs b/lib/ldb/ABI/pyldb-util-2.9.0.sigs index 164a806b2ff..218d2161cd8 100644 --- a/lib/ldb/ABI/pyldb-util-2.9.0.sigs +++ b/lib/ldb/ABI/pyldb-util-2.9.0.sigs @@ -1,3 +1,3 @@ -pyldb_Dn_FromDn: PyObject *(struct ldb_dn *) +pyldb_Dn_FromDn: PyObject *(struct ldb_dn *, PyLdbObject *) pyldb_Object_AsDn: bool (TALLOC_CTX *, PyObject *, struct ldb_context *, struct ldb_dn **) pyldb_check_type: bool (PyObject *, const char *) diff --git a/lib/ldb/pyldb.c b/lib/ldb/pyldb.c index cd4268a9a74..c16f552e21f 100644 --- a/lib/ldb/pyldb.c +++ b/lib/ldb/pyldb.c @@ -58,7 +58,7 @@ struct py_ldb_search_iterator_reply { }; void initldb(void); -static PyObject *PyLdbMessage_FromMessage(struct ldb_message *msg); +static PyObject *PyLdbMessage_FromMessage(struct ldb_message *msg, PyLdbObject *pyldb); static PyObject *PyExc_LdbError; static PyTypeObject PyLdbControl; @@ -332,7 +332,7 @@ static PyObject *PyLdbControl_FromControl(struct ldb_control *control) * @param result LDB result to convert * @return Python object with converted result (a list object) */ -static PyObject *PyLdbResult_FromResult(struct ldb_result *result) +static PyObject *PyLdbResult_FromResult(struct ldb_result *result, PyLdbObject *pyldb) { PyLdbResultObject *ret; PyObject *list, *controls, *referals; @@ -348,6 +348,9 @@ static PyObject *PyLdbResult_FromResult(struct ldb_result *result) return NULL; } + ret->pyldb = pyldb; + Py_INCREF(ret->pyldb); + list = PyList_New(result->count); if (list == NULL) { PyErr_NoMemory(); @@ -356,7 +359,7 @@ static PyObject *PyLdbResult_FromResult(struct ldb_result *result) } for (i = 0; i < result->count; i++) { - PyObject *pymessage = PyLdbMessage_FromMessage(result->msgs[i]); + PyObject *pymessage = PyLdbMessage_FromMessage(result->msgs[i], pyldb); if (pymessage == NULL) { Py_DECREF(ret); Py_DECREF(list); @@ -611,6 +614,8 @@ static PyObject *py_ldb_dn_get_parent(PyLdbDnObject *self, } py_ret->mem_ctx = mem_ctx; py_ret->dn = parent; + py_ret->pyldb = self->pyldb; + Py_INCREF(py_ret->pyldb); return (PyObject *)py_ret; } @@ -862,7 +867,7 @@ static Py_ssize_t py_ldb_dn_len(PyLdbDnObject *self) /* copy a DN as a python object */ -static PyObject *py_ldb_dn_copy(struct ldb_dn *dn) +static PyObject *py_ldb_dn_copy(struct ldb_dn *dn, PyLdbObject *pyldb) { TALLOC_CTX *mem_ctx = NULL; struct ldb_dn *new_dn = NULL; @@ -887,6 +892,9 @@ static PyObject *py_ldb_dn_copy(struct ldb_dn *dn) } py_ret->mem_ctx = mem_ctx; py_ret->dn = new_dn; + + py_ret->pyldb = pyldb; + Py_INCREF(py_ret->pyldb); return (PyObject *)py_ret; } @@ -927,6 +935,9 @@ static PyObject *py_ldb_dn_concat(PyLdbDnObject *self, PyObject *py_other) py_ret->mem_ctx = mem_ctx; py_ret->dn = new_dn; + py_ret->pyldb = self->pyldb; + Py_INCREF(py_ret->pyldb); + return (PyObject *)py_ret; } @@ -973,6 +984,8 @@ static PyObject *py_ldb_dn_new(PyTypeObject *type, PyObject *args, PyObject *kwa } py_ret->mem_ctx = mem_ctx; py_ret->dn = ret; + py_ret->pyldb = (PyLdbObject *)py_ldb; + Py_INCREF(py_ret->pyldb); out: if (str != NULL) { PyMem_Free(discard_const_p(char, str)); @@ -983,6 +996,7 @@ out: static void py_ldb_dn_dealloc(PyLdbDnObject *self) { talloc_free(self->mem_ctx); + Py_DECREF(self->pyldb); PyObject_Del(self); } @@ -1118,7 +1132,7 @@ static PyObject *py_ldb_get_root_basedn(PyLdbObject *self, struct ldb_dn *dn = ldb_get_root_basedn(pyldb_Ldb_AS_LDBCONTEXT(self)); if (dn == NULL) Py_RETURN_NONE; - return py_ldb_dn_copy(dn); + return py_ldb_dn_copy(dn, self); } @@ -1128,7 +1142,7 @@ static PyObject *py_ldb_get_schema_basedn(PyLdbObject *self, struct ldb_dn *dn = ldb_get_schema_basedn(pyldb_Ldb_AS_LDBCONTEXT(self)); if (dn == NULL) Py_RETURN_NONE; - return py_ldb_dn_copy(dn); + return py_ldb_dn_copy(dn, self); } static PyObject *py_ldb_get_config_basedn(PyLdbObject *self, @@ -1137,7 +1151,7 @@ static PyObject *py_ldb_get_config_basedn(PyLdbObject *self, struct ldb_dn *dn = ldb_get_config_basedn(pyldb_Ldb_AS_LDBCONTEXT(self)); if (dn == NULL) Py_RETURN_NONE; - return py_ldb_dn_copy(dn); + return py_ldb_dn_copy(dn, self); } static PyObject *py_ldb_get_default_basedn(PyLdbObject *self, @@ -1146,7 +1160,7 @@ static PyObject *py_ldb_get_default_basedn(PyLdbObject *self, struct ldb_dn *dn = ldb_get_default_basedn(pyldb_Ldb_AS_LDBCONTEXT(self)); if (dn == NULL) Py_RETURN_NONE; - return py_ldb_dn_copy(dn); + return py_ldb_dn_copy(dn, self); } static const char **PyList_AsStrList(TALLOC_CTX *mem_ctx, PyObject *list, @@ -1766,10 +1780,11 @@ static PyObject *py_ldb_schema_attribute_add(PyLdbObject *self, PyObject *args) Py_RETURN_NONE; } -static PyObject *ldb_ldif_to_pyobject(struct ldb_context *ldb, struct ldb_ldif *ldif) +static PyObject *ldb_ldif_to_pyobject(PyLdbObject *pyldb, struct ldb_ldif *ldif) { PyObject *obj = NULL; PyObject *result = NULL; + struct ldb_context *ldb = pyldb->ldb_ctx; if (ldif == NULL) { Py_RETURN_NONE; @@ -1778,10 +1793,10 @@ static PyObject *ldb_ldif_to_pyobject(struct ldb_context *ldb, struct ldb_ldif * switch (ldif->changetype) { case LDB_CHANGETYPE_NONE: case LDB_CHANGETYPE_ADD: - obj = PyLdbMessage_FromMessage(ldif->msg); + obj = PyLdbMessage_FromMessage(ldif->msg, pyldb); break; case LDB_CHANGETYPE_MODIFY: - obj = PyLdbMessage_FromMessage(ldif->msg); + obj = PyLdbMessage_FromMessage(ldif->msg, pyldb); break; case LDB_CHANGETYPE_DELETE: if (ldif->msg->num_elements != 0) { @@ -1790,7 +1805,7 @@ static PyObject *ldb_ldif_to_pyobject(struct ldb_context *ldb, struct ldb_ldif * ldif->msg->num_elements); return NULL; } - obj = pyldb_Dn_FromDn(ldif->msg->dn); + obj = pyldb_Dn_FromDn(ldif->msg->dn, pyldb); break; case LDB_CHANGETYPE_MODRDN: { struct ldb_dn *olddn = NULL; @@ -1815,7 +1830,7 @@ static PyObject *ldb_ldif_to_pyobject(struct ldb_context *ldb, struct ldb_ldif * return NULL; } - olddn_obj = pyldb_Dn_FromDn(olddn); + olddn_obj = pyldb_Dn_FromDn(olddn, pyldb); if (olddn_obj == NULL) { return NULL; } @@ -1824,7 +1839,7 @@ static PyObject *ldb_ldif_to_pyobject(struct ldb_context *ldb, struct ldb_ldif * } else { deleteoldrdn_obj = Py_False; } - newdn_obj = pyldb_Dn_FromDn(newdn); + newdn_obj = pyldb_Dn_FromDn(newdn, pyldb); if (newdn_obj == NULL) { deleteoldrdn_obj = NULL; Py_CLEAR(olddn_obj); @@ -1927,7 +1942,7 @@ static PyObject *py_ldb_parse_ldif(PyLdbObject *self, PyObject *args) talloc_steal(mem_ctx, ldif); if (ldif) { int res = 0; - PyObject *py_ldif = ldb_ldif_to_pyobject(self->ldb_ctx, ldif); + PyObject *py_ldif = ldb_ldif_to_pyobject(self, ldif); if (py_ldif == NULL) { Py_CLEAR(list); if (PyErr_Occurred() == NULL) { @@ -2025,7 +2040,7 @@ static PyObject *py_ldb_msg_diff(PyLdbObject *self, PyObject *args) return NULL; } - py_ret = PyLdbMessage_FromMessage(diff); + py_ret = PyLdbMessage_FromMessage(diff, self); talloc_free(mem_ctx); @@ -2184,7 +2199,7 @@ static PyObject *py_ldb_search(PyLdbObject *self, PyObject *args, PyObject *kwar return NULL; } - py_ret = PyLdbResult_FromResult(res); + py_ret = PyLdbResult_FromResult(res, self); talloc_free(mem_ctx); @@ -2234,7 +2249,7 @@ static int py_ldb_search_iterator_callback(struct ldb_request *req, switch (ares->type) { case LDB_REPLY_ENTRY: - reply->obj = PyLdbMessage_FromMessage(ares->message); + reply->obj = PyLdbMessage_FromMessage(ares->message, py_iter->ldb); if (reply->obj == NULL) { TALLOC_FREE(ares); return ldb_request_done(req, LDB_ERR_OPERATIONS_ERROR); @@ -2255,7 +2270,7 @@ static int py_ldb_search_iterator_callback(struct ldb_request *req, case LDB_REPLY_DONE: result = (struct ldb_result) { .controls = ares->controls }; - reply->obj = PyLdbResult_FromResult(&result); + reply->obj = PyLdbResult_FromResult(&result, py_iter->ldb); if (reply->obj == NULL) { TALLOC_FREE(ares); return ldb_request_done(req, LDB_ERR_OPERATIONS_ERROR); @@ -2763,6 +2778,7 @@ static void py_ldb_result_dealloc(PyLdbResultObject *self) Py_CLEAR(self->msgs); Py_CLEAR(self->referals); Py_CLEAR(self->controls); + Py_DECREF(self->pyldb); Py_TYPE(self)->tp_free(self); } @@ -3473,7 +3489,7 @@ static PyObject *py_ldb_msg_from_dict(PyTypeObject *type, PyObject *args) return NULL; } - py_ret = PyLdbMessage_FromMessage(msg); + py_ret = PyLdbMessage_FromMessage(msg, (PyLdbObject *)py_ldb); talloc_unlink(ldb_ctx, msg); @@ -3568,7 +3584,7 @@ static PyObject *py_ldb_msg_getitem(PyLdbMessageObject *self, PyObject *py_name) return NULL; } if (!ldb_attr_cmp(name, "dn")) { - return pyldb_Dn_FromDn(msg->dn); + return pyldb_Dn_FromDn(msg->dn, self->pyldb); } el = ldb_msg_find_element(msg, name); if (el == NULL) { @@ -3594,7 +3610,7 @@ static PyObject *py_ldb_msg_get(PyLdbMessageObject *self, PyObject *args, PyObje } if (strcasecmp(name, "dn") == 0) { - return pyldb_Dn_FromDn(msg->dn); + return pyldb_Dn_FromDn(msg->dn, self->pyldb); } el = ldb_msg_find_element(msg, name); @@ -3625,7 +3641,7 @@ static PyObject *py_ldb_msg_items(PyLdbMessageObject *self, } if (msg->dn != NULL) { PyObject *value = NULL; - PyObject *obj = pyldb_Dn_FromDn(msg->dn); + PyObject *obj = pyldb_Dn_FromDn(msg->dn, self->pyldb); int res = 0; value = Py_BuildValue("(sO)", "dn", obj); Py_CLEAR(obj); @@ -3878,10 +3894,14 @@ static PyObject *py_ldb_msg_new(PyTypeObject *type, PyObject *args, PyObject *kw py_ret->mem_ctx = mem_ctx; py_ret->msg = ret; + if (pydn != NULL) { + py_ret->pyldb = ((PyLdbDnObject *)pydn)->pyldb; + Py_INCREF(py_ret->pyldb); + } return (PyObject *)py_ret; } -static PyObject *PyLdbMessage_FromMessage(struct ldb_message *msg) +static PyObject *PyLdbMessage_FromMessage(struct ldb_message *msg, PyLdbObject *pyldb) { TALLOC_CTX *mem_ctx = NULL; struct ldb_message *msg_ref = NULL; @@ -3906,13 +3926,17 @@ static PyObject *PyLdbMessage_FromMessage(struct ldb_message *msg) } ret->mem_ctx = mem_ctx; ret->msg = msg_ref; + + ret->pyldb = pyldb; + Py_INCREF(ret->pyldb); + return (PyObject *)ret; } static PyObject *py_ldb_msg_get_dn(PyLdbMessageObject *self, void *closure) { struct ldb_message *msg = pyldb_Message_AsMessage(self); - return pyldb_Dn_FromDn(msg->dn); + return pyldb_Dn_FromDn(msg->dn, self->pyldb); } static int py_ldb_msg_set_dn(PyLdbMessageObject *self, PyObject *value, void *closure) @@ -3935,6 +3959,14 @@ static int py_ldb_msg_set_dn(PyLdbMessageObject *self, PyObject *value, void *cl } msg->dn = dn; + + if (self->pyldb) { + Py_DECREF(self->pyldb); + } + + self->pyldb = ((PyLdbDnObject *)value)->pyldb; + Py_INCREF(self->pyldb); + return 0; } @@ -3987,6 +4019,10 @@ static PyObject *py_ldb_msg_repr(PyLdbMessageObject *self) static void py_ldb_msg_dealloc(PyLdbMessageObject *self) { talloc_free(self->mem_ctx); + /* The pyldb element will only be present if a DN is assigned */ + if (self->pyldb) { + Py_DECREF(self->pyldb); + } PyObject_Del(self); } diff --git a/lib/ldb/pyldb.h b/lib/ldb/pyldb.h index fe5139bef5a..1c6372c2ed9 100644 --- a/lib/ldb/pyldb.h +++ b/lib/ldb/pyldb.h @@ -55,9 +55,14 @@ typedef struct { PyObject_HEAD TALLOC_CTX *mem_ctx; struct ldb_dn *dn; + /* + * We use this to keep a reference to the ldb context within + * the struct ldb_dn and to know if it is still valid + */ + PyLdbObject *pyldb; } PyLdbDnObject; -PyObject *pyldb_Dn_FromDn(struct ldb_dn *); +PyObject *pyldb_Dn_FromDn(struct ldb_dn *dn, PyLdbObject *pyldb); bool pyldb_Object_AsDn(TALLOC_CTX *mem_ctx, PyObject *object, struct ldb_context *ldb_ctx, struct ldb_dn **dn); #define pyldb_Dn_AS_DN(pyobj) ((PyLdbDnObject *)pyobj)->dn @@ -74,6 +79,12 @@ typedef struct { PyObject_HEAD TALLOC_CTX *mem_ctx; struct ldb_message *msg; + /* + * We use this to keep a reference to the ldb context within + * the struct ldb_dn (under struct ldb_message) and to know if + * it is still valid + */ + PyLdbObject *pyldb; } PyLdbMessageObject; #define pyldb_Message_AsMessage(pyobj) ((PyLdbMessageObject *)pyobj)->msg @@ -104,6 +115,7 @@ typedef struct { PyObject *msgs; PyObject *referals; PyObject *controls; + PyLdbObject *pyldb; } PyLdbResultObject; typedef struct { diff --git a/lib/ldb/pyldb_util.c b/lib/ldb/pyldb_util.c index 87407f0eaa6..44bc9941d29 100644 --- a/lib/ldb/pyldb_util.c +++ b/lib/ldb/pyldb_util.c @@ -145,7 +145,7 @@ bool pyldb_Object_AsDn(TALLOC_CTX *mem_ctx, PyObject *object, return false; } -PyObject *pyldb_Dn_FromDn(struct ldb_dn *dn) +PyObject *pyldb_Dn_FromDn(struct ldb_dn *dn, PyLdbObject *pyldb) { TALLOC_CTX *mem_ctx = NULL; struct ldb_dn *dn_ref = NULL; @@ -182,6 +182,9 @@ PyObject *pyldb_Dn_FromDn(struct ldb_dn *dn) } py_ret->mem_ctx = mem_ctx; py_ret->dn = dn; + py_ret->pyldb = pyldb; + + Py_INCREF(py_ret->pyldb); return (PyObject *)py_ret; } diff --git a/selftest/knownfail.d/ldb-use-after-free b/selftest/knownfail.d/ldb-use-after-free deleted file mode 100644 index 5a3bda7599b..00000000000 --- a/selftest/knownfail.d/ldb-use-after-free +++ /dev/null @@ -1,11 +0,0 @@ -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_concat -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_dn_assign -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_get_default_basedn -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_get_ncroot_existing -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_get_ncroot_not_existing -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_get_parent -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_get_wellknown_dn -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_msg_diff -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_msg_from_dict -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_free_search_result -^samba.tests.segfault.samba.tests.segfault.SegfaultTests.test_ldb_use_after_ldif_parse diff --git a/source4/dns_server/pydns.c b/source4/dns_server/pydns.c index 81aa6b7488c..67f72979fbc 100644 --- a/source4/dns_server/pydns.c +++ b/source4/dns_server/pydns.c @@ -142,7 +142,7 @@ static PyObject *py_dsdb_dns_lookup(PyObject *self, } ret = py_dnsp_DnssrvRpcRecord_get_list(records, num_records); - pydn = pyldb_Dn_FromDn(dn); + pydn = pyldb_Dn_FromDn(dn, (PyLdbObject *)py_ldb); talloc_free(frame); result = Py_BuildValue("(OO)", pydn, ret); Py_CLEAR(ret); diff --git a/source4/dsdb/pydsdb.c b/source4/dsdb/pydsdb.c index 5d94fdd9c9a..2ae569b2749 100644 --- a/source4/dsdb/pydsdb.c +++ b/source4/dsdb/pydsdb.c @@ -977,7 +977,7 @@ static PyObject *py_dsdb_get_partitions_dn(PyObject *self, PyObject *args) PyErr_NoMemory(); return NULL; } - ret = pyldb_Dn_FromDn(dn); + ret = pyldb_Dn_FromDn(dn, (PyLdbObject *)py_ldb); talloc_free(dn); return ret; } @@ -999,7 +999,7 @@ static PyObject *py_dsdb_get_nc_root(PyObject *self, PyObject *args) ret = dsdb_find_nc_root(ldb, ldb, dn, &nc_root); PyErr_LDB_ERROR_IS_ERR_RAISE(py_ldb_get_exception(), ret, ldb); - py_nc_root = pyldb_Dn_FromDn(nc_root); + py_nc_root = pyldb_Dn_FromDn(nc_root, (PyLdbObject *)py_ldb); talloc_unlink(ldb, nc_root); return py_nc_root; } @@ -1026,7 +1026,7 @@ static PyObject *py_dsdb_get_wellknown_dn(PyObject *self, PyObject *args) PyErr_LDB_ERROR_IS_ERR_RAISE(py_ldb_get_exception(), ret, ldb); - py_wk_dn = pyldb_Dn_FromDn(wk_dn); + py_wk_dn = pyldb_Dn_FromDn(wk_dn, (PyLdbObject *)py_ldb); talloc_unlink(ldb, wk_dn); return py_wk_dn; } @@ -1396,7 +1396,8 @@ static PyObject *py_dsdb_create_gkdi_root_key(PyObject *self, PyObject *args, Py samdb, tmp_ctx); - py_dn = pyldb_Dn_FromDn(root_key_msg->dn); + py_dn = pyldb_Dn_FromDn(root_key_msg->dn, + (PyLdbObject *)py_ldb); if (py_dn == NULL) { PyErr_LDB_ERROR_IS_ERR_RAISE_FREE(py_ldb_get_exception(), LDB_ERR_OPERATIONS_ERROR, -- 2.34.1