mirror of
https://github.com/samba-team/samba.git
synced 2024-12-22 13:34:15 +03:00
CVE-2023-0614 ldb: Make use of ldb_filter_attrs_in_place()
Change all uses of ldb_kv_filter_attrs() to use ldb_filter_attrs_in_place() instead. This function does less work than its predecessor, and no longer requires the allocation of a second ldb message. Some of the work is able to be split out into separate functions that each accomplish a single task, with a purpose to make the code clearer. BUG: https://bugzilla.samba.org/show_bug.cgi?id=15270 Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
This commit is contained in:
parent
f25b1756aa
commit
fffea59001
@ -301,10 +301,8 @@ int ldb_kv_search_key(struct ldb_module *module,
|
||||
const struct ldb_val ldb_key,
|
||||
struct ldb_message *msg,
|
||||
unsigned int unpack_flags);
|
||||
int ldb_kv_filter_attrs(struct ldb_context *ldb,
|
||||
const struct ldb_message *msg,
|
||||
const char *const *attrs,
|
||||
struct ldb_message *filtered_msg);
|
||||
int ldb_kv_filter_attrs_in_place(struct ldb_message *msg,
|
||||
const char *const *attrs);
|
||||
int ldb_kv_search(struct ldb_kv_context *ctx);
|
||||
|
||||
/*
|
||||
|
@ -2264,7 +2264,6 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv,
|
||||
{
|
||||
struct ldb_context *ldb = ldb_module_get_ctx(ac->module);
|
||||
struct ldb_message *msg;
|
||||
struct ldb_message *filtered_msg;
|
||||
unsigned int i;
|
||||
unsigned int num_keys = 0;
|
||||
uint8_t previous_guid_key[LDB_KV_GUID_KEY_SIZE] = {0};
|
||||
@ -2456,27 +2455,31 @@ static int ldb_kv_index_filter(struct ldb_kv_private *ldb_kv,
|
||||
continue;
|
||||
}
|
||||
|
||||
filtered_msg = ldb_msg_new(ac);
|
||||
if (filtered_msg == NULL) {
|
||||
TALLOC_FREE(keys);
|
||||
TALLOC_FREE(msg);
|
||||
ret = ldb_msg_add_distinguished_name(msg);
|
||||
if (ret == -1) {
|
||||
talloc_free(msg);
|
||||
return LDB_ERR_OPERATIONS_ERROR;
|
||||
}
|
||||
|
||||
filtered_msg->dn = talloc_steal(filtered_msg, msg->dn);
|
||||
|
||||
/* filter the attributes that the user wants */
|
||||
ret = ldb_kv_filter_attrs(ldb, msg, ac->attrs, filtered_msg);
|
||||
|
||||
talloc_free(msg);
|
||||
|
||||
if (ret == -1) {
|
||||
TALLOC_FREE(filtered_msg);
|
||||
ret = ldb_kv_filter_attrs_in_place(msg, ac->attrs);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
talloc_free(keys);
|
||||
talloc_free(msg);
|
||||
return LDB_ERR_OPERATIONS_ERROR;
|
||||
}
|
||||
|
||||
ret = ldb_module_send_entry(ac->req, filtered_msg, NULL);
|
||||
ldb_msg_shrink_to_fit(msg);
|
||||
|
||||
/* Ensure the message elements are all talloc'd. */
|
||||
ret = ldb_msg_elements_take_ownership(msg);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
talloc_free(keys);
|
||||
talloc_free(msg);
|
||||
return LDB_ERR_OPERATIONS_ERROR;
|
||||
}
|
||||
|
||||
ret = ldb_module_send_entry(ac->req, msg, NULL);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
/* Regardless of success or failure, the msg
|
||||
* is the callbacks responsibility, and should
|
||||
|
@ -293,15 +293,13 @@ int ldb_kv_search_dn1(struct ldb_module *module,
|
||||
|
||||
/*
|
||||
* filter the specified list of attributes from msg,
|
||||
* adding requested attributes, and perhaps all for *,
|
||||
* but not the DN to filtered_msg.
|
||||
* adding requested attributes, and perhaps all for *.
|
||||
* The DN will not be added if it is missing.
|
||||
*/
|
||||
int ldb_kv_filter_attrs(struct ldb_context *ldb,
|
||||
const struct ldb_message *msg,
|
||||
const char *const *attrs,
|
||||
struct ldb_message *filtered_msg)
|
||||
int ldb_kv_filter_attrs_in_place(struct ldb_message *msg,
|
||||
const char *const *attrs)
|
||||
{
|
||||
return ldb_filter_attrs(ldb, msg, attrs, filtered_msg);
|
||||
return ldb_filter_attrs_in_place(msg, attrs);
|
||||
}
|
||||
|
||||
/*
|
||||
@ -314,7 +312,7 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
|
||||
{
|
||||
struct ldb_context *ldb;
|
||||
struct ldb_kv_context *ac;
|
||||
struct ldb_message *msg, *filtered_msg;
|
||||
struct ldb_message *msg;
|
||||
struct timeval now;
|
||||
int ret, timeval_cmp;
|
||||
bool matched;
|
||||
@ -411,25 +409,31 @@ static int search_func(_UNUSED_ struct ldb_kv_private *ldb_kv,
|
||||
return 0;
|
||||
}
|
||||
|
||||
filtered_msg = ldb_msg_new(ac);
|
||||
if (filtered_msg == NULL) {
|
||||
TALLOC_FREE(msg);
|
||||
ret = ldb_msg_add_distinguished_name(msg);
|
||||
if (ret == -1) {
|
||||
talloc_free(msg);
|
||||
return LDB_ERR_OPERATIONS_ERROR;
|
||||
}
|
||||
|
||||
filtered_msg->dn = talloc_steal(filtered_msg, msg->dn);
|
||||
|
||||
/* filter the attributes that the user wants */
|
||||
ret = ldb_kv_filter_attrs(ldb, msg, ac->attrs, filtered_msg);
|
||||
talloc_free(msg);
|
||||
|
||||
if (ret == -1) {
|
||||
TALLOC_FREE(filtered_msg);
|
||||
ret = ldb_kv_filter_attrs_in_place(msg, ac->attrs);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
talloc_free(msg);
|
||||
ac->error = LDB_ERR_OPERATIONS_ERROR;
|
||||
return -1;
|
||||
}
|
||||
|
||||
ret = ldb_module_send_entry(ac->req, filtered_msg, NULL);
|
||||
ldb_msg_shrink_to_fit(msg);
|
||||
|
||||
/* Ensure the message elements are all talloc'd. */
|
||||
ret = ldb_msg_elements_take_ownership(msg);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
talloc_free(msg);
|
||||
ac->error = LDB_ERR_OPERATIONS_ERROR;
|
||||
return -1;
|
||||
}
|
||||
|
||||
ret = ldb_module_send_entry(ac->req, msg, NULL);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
ac->request_terminated = true;
|
||||
/* the callback failed, abort the operation */
|
||||
@ -492,7 +496,7 @@ static int ldb_kv_search_full(struct ldb_kv_context *ctx)
|
||||
static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
|
||||
struct ldb_kv_context *ctx)
|
||||
{
|
||||
struct ldb_message *msg, *filtered_msg;
|
||||
struct ldb_message *msg;
|
||||
struct ldb_context *ldb = ldb_module_get_ctx(ctx->module);
|
||||
const char *dn_linearized;
|
||||
const char *msg_dn_linearized;
|
||||
@ -550,12 +554,6 @@ static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
|
||||
dn_linearized = ldb_dn_get_linearized(ctx->base);
|
||||
msg_dn_linearized = ldb_dn_get_linearized(msg->dn);
|
||||
|
||||
filtered_msg = ldb_msg_new(ctx);
|
||||
if (filtered_msg == NULL) {
|
||||
talloc_free(msg);
|
||||
return LDB_ERR_OPERATIONS_ERROR;
|
||||
}
|
||||
|
||||
if (strcmp(dn_linearized, msg_dn_linearized) == 0) {
|
||||
/*
|
||||
* If the DN is exactly the same string, then
|
||||
@ -563,36 +561,42 @@ static int ldb_kv_search_and_return_base(struct ldb_kv_private *ldb_kv,
|
||||
* returned result, as it has already been
|
||||
* casefolded
|
||||
*/
|
||||
filtered_msg->dn = ldb_dn_copy(filtered_msg, ctx->base);
|
||||
struct ldb_dn *dn = ldb_dn_copy(msg, ctx->base);
|
||||
if (dn != NULL) {
|
||||
msg->dn = dn;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* If the ldb_dn_copy() failed, or if we did not choose that
|
||||
* optimisation (filtered_msg is zeroed at allocation),
|
||||
* steal the one from the unpack
|
||||
*/
|
||||
if (filtered_msg->dn == NULL) {
|
||||
filtered_msg->dn = talloc_steal(filtered_msg, msg->dn);
|
||||
ret = ldb_msg_add_distinguished_name(msg);
|
||||
if (ret == -1) {
|
||||
talloc_free(msg);
|
||||
return LDB_ERR_OPERATIONS_ERROR;
|
||||
}
|
||||
|
||||
/*
|
||||
* filter the attributes that the user wants.
|
||||
*/
|
||||
ret = ldb_kv_filter_attrs(ldb, msg, ctx->attrs, filtered_msg);
|
||||
if (ret == -1) {
|
||||
ret = ldb_kv_filter_attrs_in_place(msg, ctx->attrs);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
talloc_free(msg);
|
||||
return LDB_ERR_OPERATIONS_ERROR;
|
||||
}
|
||||
|
||||
ldb_msg_shrink_to_fit(msg);
|
||||
|
||||
/* Ensure the message elements are all talloc'd. */
|
||||
ret = ldb_msg_elements_take_ownership(msg);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
talloc_free(msg);
|
||||
filtered_msg = NULL;
|
||||
return LDB_ERR_OPERATIONS_ERROR;
|
||||
}
|
||||
|
||||
/*
|
||||
* Remove any extended components possibly copied in from
|
||||
* msg->dn, we just want the casefold components
|
||||
* Remove any extended components, we just want the casefold components
|
||||
*/
|
||||
ldb_dn_remove_extended_components(filtered_msg->dn);
|
||||
talloc_free(msg);
|
||||
ldb_dn_remove_extended_components(msg->dn);
|
||||
|
||||
ret = ldb_module_send_entry(ctx->req, filtered_msg, NULL);
|
||||
ret = ldb_module_send_entry(ctx->req, msg, NULL);
|
||||
if (ret != LDB_SUCCESS) {
|
||||
/* Regardless of success or failure, the msg
|
||||
* is the callbacks responsibility, and should
|
||||
|
@ -1634,7 +1634,6 @@ static bool torture_ldb_unpack_and_filter(struct torture_context *torture,
|
||||
TALLOC_CTX *mem_ctx = talloc_new(torture);
|
||||
struct ldb_context *ldb;
|
||||
struct ldb_val data = *discard_const_p(struct ldb_val, data_p);
|
||||
struct ldb_message *unpack_msg = ldb_msg_new(mem_ctx);
|
||||
struct ldb_message *msg = ldb_msg_new(mem_ctx);
|
||||
const char *lookup_names[] = {"instanceType", "nonexistent",
|
||||
"whenChanged", "objectClass",
|
||||
@ -1649,18 +1648,15 @@ static bool torture_ldb_unpack_and_filter(struct torture_context *torture,
|
||||
"Failed to init samba");
|
||||
|
||||
torture_assert_int_equal(torture,
|
||||
ldb_unpack_data(ldb, &data, unpack_msg),
|
||||
ldb_unpack_data(ldb, &data, msg),
|
||||
0, "ldb_unpack_data failed");
|
||||
|
||||
torture_assert_int_equal(torture, unpack_msg->num_elements, 13,
|
||||
torture_assert_int_equal(torture, msg->num_elements, 13,
|
||||
"Got wrong count of elements");
|
||||
|
||||
msg->dn = talloc_steal(msg, unpack_msg->dn);
|
||||
|
||||
torture_assert_int_equal(torture,
|
||||
ldb_filter_attrs(ldb, unpack_msg,
|
||||
lookup_names, msg),
|
||||
0, "ldb_kv_filter_attrs failed");
|
||||
ldb_filter_attrs_in_place(msg, lookup_names),
|
||||
0, "ldb_filter_attrs_in_place failed");
|
||||
|
||||
/* Compare data in binary form */
|
||||
torture_assert_int_equal(torture, msg->num_elements, 6,
|
||||
|
Loading…
Reference in New Issue
Block a user