>From 3ff06a7f1582767da5a32d5da6214a2f8a92de3e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Fri, 28 Nov 2014 19:46:38 +0100 Subject: [PATCH] SYSDB: sysdb_search_object_by_sid should return ENOENT There was a crash in function get_object_from_cache because sysdb_search_object_by_sid didn' returned ENOENT and res->msgs was NULL (dereference of NULL pointer) Explanation Resolves: https://fedorahosted.org/sssd/ticket/XXXX --- src/db/sysdb.h | 2 +- src/db/sysdb_ops.c | 53 +----------------------------------------- src/responder/nss/nsssrv_cmd.c | 24 +++++++++---------- src/responder/pac/pacsrv_cmd.c | 12 ++++++---- src/tests/sysdb-tests.c | 7 ++---- 5 files changed, 23 insertions(+), 75 deletions(-) diff --git a/src/db/sysdb.h b/src/db/sysdb.h index 5bd7f90acb685bbaff5c98f433c7dce8175c33ca..9b88b4a63619456f8f3cc1961e29bbf3946ca5b8 100644 --- a/src/db/sysdb.h +++ b/src/db/sysdb.h @@ -1033,7 +1033,7 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, const char *sid_str, const char **attrs, - struct ldb_result **msg); + struct ldb_result **res); errno_t sysdb_search_object_by_uuid(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c index 998046a2ca1c746b2032f430e5f9c4a7151e1dbc..e88e41f5bfbe0fb8f0875a170d5b55473f18d111 100644 --- a/src/db/sysdb_ops.c +++ b/src/db/sysdb_ops.c @@ -3564,61 +3564,10 @@ errno_t sysdb_search_object_by_sid(TALLOC_CTX *mem_ctx, struct sss_domain_info *domain, const char *sid_str, const char **attrs, - struct ldb_result **msg) + struct ldb_result **res) { -/* TODO: use return sysdb_search_object_by_str_attr(mem_ctx, domain, SYSDB_SID_FILTER, sid_str, attrs, res); - - when verified that all callers can handle ENOENT correctly. */ - - TALLOC_CTX *tmp_ctx; - const char *def_attrs[] = { SYSDB_NAME, SYSDB_UIDNUM, SYSDB_GIDNUM, - ORIGINALAD_PREFIX SYSDB_NAME, - SYSDB_OBJECTCLASS, NULL }; - struct ldb_dn *basedn; - int ret; - struct ldb_result *res = NULL; - - tmp_ctx = talloc_new(NULL); - if (!tmp_ctx) { - return ENOMEM; - } - - basedn = ldb_dn_new_fmt(tmp_ctx, domain->sysdb->ldb, SYSDB_DOM_BASE, domain->name); - if (basedn == NULL) { - DEBUG(SSSDBG_OP_FAILURE, "ldb_dn_new_fmt failed.\n"); - ret = ENOMEM; - goto done; - } - - ret = ldb_search(domain->sysdb->ldb, tmp_ctx, &res, - basedn, LDB_SCOPE_SUBTREE, attrs?attrs:def_attrs, - SYSDB_SID_FILTER, sid_str); - if (ret != EOK) { - ret = sysdb_error_to_errno(ret); - DEBUG(SSSDBG_OP_FAILURE, "ldb_search failed.\n"); - goto done; - } - - if (res->count > 1) { - DEBUG(SSSDBG_CRIT_FAILURE, "Search for SID [%s] returned more than " \ - "one object.\n", sid_str); - ret = EINVAL; - goto done; - } - - *msg = talloc_steal(mem_ctx, res); - -done: - if (ret == ENOENT) { - DEBUG(SSSDBG_TRACE_FUNC, "No such entry.\n"); - } else if (ret) { - DEBUG(SSSDBG_OP_FAILURE, "Error: %d (%s)\n", ret, strerror(ret)); - } - - talloc_zfree(tmp_ctx); - return ret; } errno_t sysdb_search_object_by_uuid(TALLOC_CTX *mem_ctx, diff --git a/src/responder/nss/nsssrv_cmd.c b/src/responder/nss/nsssrv_cmd.c index ea58920bc3611c84958e8d0aca0e122d90c68e5c..b6b4a947443aec4b9219004bc5e305415bb8d5d7 100644 --- a/src/responder/nss/nsssrv_cmd.c +++ b/src/responder/nss/nsssrv_cmd.c @@ -4491,18 +4491,7 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx) ret = sysdb_search_object_by_sid(cmdctx, dom, cmdctx->secid, NULL, &dctx->res); - if (ret != EOK) { - DEBUG(SSSDBG_CRIT_FAILURE, "Failed to make request to our cache!\n"); - return EIO; - } - - if (dctx->res->count > 1) { - DEBUG(SSSDBG_FATAL_FAILURE, "getbysid call returned more than one " \ - "result !?!\n"); - return ENOENT; - } - - if (dctx->res->count == 0 && !dctx->check_provider) { + if (ret == ENOENT && !dctx->check_provider) { DEBUG(SSSDBG_OP_FAILURE, "No results for getbysid call.\n"); /* set negative cache only if not result of cache check */ @@ -4515,6 +4504,17 @@ static errno_t nss_cmd_getbysid_search(struct nss_dom_ctx *dctx) return ENOENT; } + if (ret != EOK) { + DEBUG(SSSDBG_CRIT_FAILURE, "Failed to make request to our cache!\n"); + return EIO; + } + + if (dctx->res->count > 1) { + DEBUG(SSSDBG_FATAL_FAILURE, "getbysid call returned more than one " + "result !?!\n"); + return ENOENT; + } + /* if this is a caching provider (or if we haven't checked the cache * yet) then verify that the cache is uptodate */ if (dctx->check_provider) { diff --git a/src/responder/pac/pacsrv_cmd.c b/src/responder/pac/pacsrv_cmd.c index cc92592893899b1fa269188facc1a8154f80991d..c089c3cbdd52d059c2a2e18e32cc78a702e9a462 100644 --- a/src/responder/pac/pacsrv_cmd.c +++ b/src/responder/pac/pacsrv_cmd.c @@ -297,17 +297,19 @@ static void pac_lookup_sids_done(struct tevent_req *req) msg = NULL; ret = sysdb_search_object_by_sid(pr_ctx, dom, entries[c].key.str, NULL, &msg); + if (ret == ENOENT) { + DEBUG(SSSDBG_OP_FAILURE, "No entry found for SID [%s].\n", + entries[c].key.str); + continue; + } + if (ret != EOK) { DEBUG(SSSDBG_OP_FAILURE, "sysdb_search_object_by_sid " \ "failed.\n"); continue; } - if (msg->count == 0) { - DEBUG(SSSDBG_OP_FAILURE, "No entry found for SID [%s].\n", - entries[c].key.str); - continue; - } else if (msg->count > 1) { + if (msg->count > 1) { DEBUG(SSSDBG_CRIT_FAILURE, "More then one result returned " \ "for SID [%s].\n", entries[c].key.str); diff --git a/src/tests/sysdb-tests.c b/src/tests/sysdb-tests.c index 26835d9eb3647c99a8cf50d98143088dc0655f10..a1a5fc0542c35c4af121e66ee4c52c0a509e7487 100644 --- a/src/tests/sysdb-tests.c +++ b/src/tests/sysdb-tests.c @@ -4809,13 +4809,10 @@ START_TEST (test_sysdb_search_return_ENOENT) talloc_zfree(res); /* Search object */ - /* TODO: Should return ENOENT */ ret = sysdb_search_object_by_sid(test_ctx, test_ctx->domain, "S-5-4-3-2-1", NULL, &res); - fail_unless(ret == EOK, "sysdb_search_object_by_sid_str failed with " - "[%d][%s].", ret, strerror(ret)); - fail_unless(res->count == 0, "sysdb_search_object_by_sid_str should not " - "return anything."); + fail_unless(ret == ENOENT, "sysdb_search_object_by_sid_str failed with " + "[%d][%s].", ret, strerror(ret)); talloc_zfree(res); /* Search can return more results */ -- 2.1.0