URL: https://github.com/SSSD/sssd/pull/5709 Author: elkoniu Title: #5709: General: Hardeninig getenv() usage Action: opened
PR body: """ Pointer returned by getenv() should be cached locally before it is passed down to sub functions. This PR fixes this for: * pam_sm_authenticate() * sysdb_ldb_connect() * files_init_file_sources() Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1977830 """ To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5709/head:pr5709 git checkout pr5709
From fff571c34dec909b882a0018327c51b06b025050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Po=C5=82awski?= <ppola...@redhat.com> Date: Tue, 13 Jul 2021 00:40:30 +0200 Subject: [PATCH] General: Hardeninig getenv() usage Pointer returned by getenv() should be cached locally before it is passed down to sub functions. This PR fixes this for: * pam_sm_authenticate() * sysdb_ldb_connect() * files_init_file_sources() Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1977830 --- src/db/sysdb_init.c | 28 ++++++++++++++++++++++------ src/providers/files/files_init.c | 6 ++++++ src/sss_client/pam_sss_gss.c | 17 ++++++++++++----- 3 files changed, 40 insertions(+), 11 deletions(-) diff --git a/src/db/sysdb_init.c b/src/db/sysdb_init.c index 1f47e03a35..3be15dc06a 100644 --- a/src/db/sysdb_init.c +++ b/src/db/sysdb_init.c @@ -51,25 +51,36 @@ errno_t sysdb_ldb_connect(TALLOC_CTX *mem_ctx, int flags, struct ldb_context **_ldb) { + TALLOC_CTX *tmp_ctx = NULL; int ret; struct ldb_context *ldb; const char *mod_path; + tmp_ctx = talloc_new(NULL); + if (tmp_ctx == NULL) { + ret = ENOMEM; + goto done; + } + if (_ldb == NULL) { - return EINVAL; + ret = EINVAL; + goto done; } ldb = ldb_init(mem_ctx, NULL); if (!ldb) { - return EIO; + ret = EIO; + goto done; } ret = ldb_set_debug(ldb, ldb_debug_messages, NULL); if (ret != LDB_SUCCESS) { - return EIO; + ret = EIO; + goto done; } - mod_path = getenv(LDB_MODULES_PATH); + /* getenv() returns pointer to external memory, which may change */ + mod_path = talloc_strdup(tmp_ctx, getenv(LDB_MODULES_PATH)); if (mod_path != NULL) { DEBUG(SSSDBG_TRACE_ALL, "Setting ldb module path to [%s].\n", mod_path); ldb_set_modules_dir(ldb, mod_path); @@ -77,12 +88,17 @@ errno_t sysdb_ldb_connect(TALLOC_CTX *mem_ctx, ret = ldb_connect(ldb, filename, flags, NULL); if (ret != LDB_SUCCESS) { - return EIO; + ret = EIO; + goto done; } *_ldb = ldb; - return EOK; + ret = EOK; + +done: + talloc_free(tmp_ctx); + return ret; } static errno_t sysdb_ldb_reconnect(TALLOC_CTX *mem_ctx, diff --git a/src/providers/files/files_init.c b/src/providers/files/files_init.c index 1ce4bcf272..7926787138 100644 --- a/src/providers/files/files_init.c +++ b/src/providers/files/files_init.c @@ -60,6 +60,9 @@ static errno_t files_init_file_sources(TALLOC_CTX *mem_ctx, } else { dfl_passwd_files = DEFAULT_PASSWD_FILE; } + + /* getenv() returns pointer to external memory, which may change */ + dfl_passwd_files = talloc_strdup(tmp_ctx, dfl_passwd_files); DEBUG(SSSDBG_TRACE_FUNC, "Using default passwd file: [%s].\n", dfl_passwd_files); @@ -72,6 +75,9 @@ static errno_t files_init_file_sources(TALLOC_CTX *mem_ctx, } else { env_group_files = DEFAULT_GROUP_FILE; } + + /* getenv() returns pointer to external memory, which may change */ + env_group_files = talloc_strdup(tmp_ctx, env_group_files); DEBUG(SSSDBG_TRACE_FUNC, "Using default group file: [%s].\n", DEFAULT_GROUP_FILE); diff --git a/src/sss_client/pam_sss_gss.c b/src/sss_client/pam_sss_gss.c index 51be36ecea..494f90b8bd 100644 --- a/src/sss_client/pam_sss_gss.c +++ b/src/sss_client/pam_sss_gss.c @@ -30,6 +30,7 @@ #include <sys/types.h> #include <sys/syslog.h> #include <unistd.h> +#include <string.h> #include "util/sss_format.h" #include "sss_client/sss_cli.h" @@ -492,9 +493,9 @@ int pam_sm_authenticate(pam_handle_t *pamh, int argc, const char **argv) { - const char *pam_service; - const char *pam_user; - const char *ccache; + const char *pam_service = NULL; + const char *pam_user = NULL; + const char *ccache = NULL; char *username = NULL; char *domain = NULL; char *target = NULL; @@ -511,9 +512,14 @@ int pam_sm_authenticate(pam_handle_t *pamh, } } - /* Get non-default ccache if specified, may be NULL. */ - ccache = getenv("KRB5CCNAME"); + /* getenv() returns pointer to external memory, which may change */ + ccache = strdup(getenv("KRB5CCNAME")); + if (NULL == ccache) { + ERROR(pamh, "Out of memory!"); + ret = ENOMEM; + goto done; + } uid = getuid(); euid = geteuid(); @@ -566,6 +572,7 @@ int pam_sm_authenticate(pam_handle_t *pamh, free(domain); free(target); free(upn); + free(ccache); return errno_to_pam(pamh, ret); }
_______________________________________________ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/sssd-devel@lists.fedorahosted.org Do not reply to spam on the list, report it: https://pagure.io/fedora-infrastructure
