>From a18938ac0f0c1bc259aed527ce34f274d0fd0473 Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik Date: Mon, 14 Oct 2013 11:21:02 +0200 Subject: [PATCH 2/2] krb5: Check return value of sss_krb5_princ_realm sss_krb5_princ_realm set output parameter realm to NULL and len to 0 in case of failure. Clang static analysers reported warning "Null pointer passed as an argument to a 'nonnull' parameter" in function match_principal. It was possible, that realm_name with value NULL could be used in strncmp. --- src/providers/krb5/krb5_child.c | 8 ++++++++ src/providers/krb5/krb5_utils.c | 5 +++++ src/util/sss_krb5.c | 13 +++++++++++++ 3 files changed, 26 insertions(+) diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c index 945f4c76ec48b594a8033438251cb0d913ddcf0c..0aa7abb8283956c7eb9d36a2b1ebaee0ae4a26c6 100644 --- a/src/providers/krb5/krb5_child.c +++ b/src/providers/krb5/krb5_child.c @@ -969,6 +969,10 @@ static krb5_error_code get_and_save_tgt(struct krb5_req *kr, } sss_krb5_princ_realm(kr->ctx, kr->princ, &realm_name, &realm_length); + if (realm_length == 0) { + DEBUG(SSSDBG_CRIT_FAILURE, "sss_krb5_princ_realm failed.\n"); + return KRB5KRB_ERR_GENERIC; + } DEBUG(SSSDBG_TRACE_FUNC, "Attempting kinit for realm [%s]\n",realm_name); @@ -1112,6 +1116,10 @@ static errno_t changepw_child(struct krb5_req *kr, bool prelim) set_changepw_options(kr->options); sss_krb5_princ_realm(kr->ctx, kr->princ, &realm_name, &realm_length); + if (realm_length == 0) { + DEBUG(SSSDBG_CRIT_FAILURE, "sss_krb5_princ_realm failed.\n"); + return ERR_INTERNAL; + } DEBUG(SSSDBG_TRACE_FUNC, "Attempting kinit for realm [%s]\n",realm_name); diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c index 0d2e281198390043068e21b412b57de04df6a12b..4f578e4df9fe5b87781e02555c619e4445196202 100644 --- a/src/providers/krb5/krb5_utils.c +++ b/src/providers/krb5/krb5_utils.c @@ -653,6 +653,11 @@ errno_t get_ccache_file_data(const char *ccache_file, const char *client_name, } sss_krb5_princ_realm(ctx, client_princ, &realm_name, &realm_length); + if (realm_length == 0) { + kerr = KRB5KRB_ERR_GENERIC; + DEBUG(SSSDBG_CRIT_FAILURE, "sss_krb5_princ_realm failed.\n"); + goto done; + } server_name = talloc_asprintf(NULL, "krbtgt/%.*s@%.*s", realm_length, realm_name, diff --git a/src/util/sss_krb5.c b/src/util/sss_krb5.c index 7daedb47464917966350f94c7d5b0844ccee7edb..0cfb3873bd5a5d94c704f98ffd7388ead497b2f5 100644 --- a/src/util/sss_krb5.c +++ b/src/util/sss_krb5.c @@ -212,6 +212,15 @@ errno_t select_principal_from_keytab(TALLOC_CTX *mem_ctx, sss_krb5_princ_realm(krb_ctx, client_princ, &realm_name, &realm_len); + if (realm_len == 0) { + DEBUG(SSSDBG_CRIT_FAILURE, "sss_krb5_princ_realm failed.\n"); + if (_principal) talloc_zfree(*_principal); + if (_primary) talloc_zfree(*_primary); + ret = EINVAL; + goto done; + + } + *_realm = talloc_asprintf(mem_ctx, "%.*s", realm_len, realm_name); if (!*_realm) { @@ -355,6 +364,10 @@ static bool match_principal(krb5_context ctx, bool ret = false; sss_krb5_princ_realm(ctx, principal, &realm_name, &realm_len); + if (realm_len == 0) { + DEBUG(SSSDBG_MINOR_FAILURE, "sss_krb5_princ_realm failed.\n"); + return false; + } tmp_ctx = talloc_new(NULL); if (!tmp_ctx) { -- 2.1.0