URL: https://github.com/SSSD/sssd/pull/5498
Author: abbra
Title: #5498: Covscan fixes
Action: opened
PR body:
"""
"""
To pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5498/head:pr5498
git checkout pr5498
From 7a6cc2f05bd33b43d27e02489290f8d217e8ab36 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Tue, 9 Feb 2021 21:52:19 +0200
Subject: [PATCH 1/4] prompt config: fix covscan errors
Covscan is confused by dangling pointers in arrays after freeing. Its
analyzer may decide to visit already visited list elements and since
they weren't NULL-ed, it may consider double-free to happen in the code.
Signed-off-by: Alexander Bokovoy <aboko...@redhat.com>
---
src/sss_client/pam_sss_prompt_config.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/sss_client/pam_sss_prompt_config.c b/src/sss_client/pam_sss_prompt_config.c
index 35094b4068..1c67fb18fd 100644
--- a/src/sss_client/pam_sss_prompt_config.c
+++ b/src/sss_client/pam_sss_prompt_config.c
@@ -98,6 +98,7 @@ static void pc_free_password(struct prompt_config *pc)
{
if (pc != NULL && pc_get_type(pc) == PC_TYPE_PASSWORD) {
free(pc->data.password.prompt);
+ pc->data.password.prompt = NULL;
}
return;
}
@@ -106,7 +107,9 @@ static void pc_free_2fa(struct prompt_config *pc)
{
if (pc != NULL && pc_get_type(pc) == PC_TYPE_2FA) {
free(pc->data.two_fa.prompt_1st);
+ pc->data.two_fa.prompt_1st = NULL;
free(pc->data.two_fa.prompt_2nd);
+ pc->data.two_fa.prompt_2nd = NULL;
}
return;
}
@@ -115,6 +118,7 @@ static void pc_free_2fa_single(struct prompt_config *pc)
{
if (pc != NULL && pc_get_type(pc) == PC_TYPE_2FA_SINGLE) {
free(pc->data.two_fa_single.prompt);
+ pc->data.two_fa_single.prompt = NULL;
}
return;
}
@@ -123,6 +127,7 @@ static void pc_free_sc_pin(struct prompt_config *pc)
{
if (pc != NULL && pc_get_type(pc) == PC_TYPE_SC_PIN) {
free(pc->data.sc_pin.prompt);
+ pc->data.sc_pin.prompt = NULL;
}
return;
}
@@ -153,6 +158,7 @@ void pc_list_free(struct prompt_config **pc_list)
return;
}
free(pc_list[c]);
+ pc_list[c] = NULL;
}
free(pc_list);
}
@@ -541,6 +547,7 @@ errno_t pc_list_from_response(int size, uint8_t *buf,
done:
if (ret != EOK) {
pc_list_free(pl);
+ pl = NULL;
}
return ret;
From 0f65a3900c0695598203484f1f3f92bef3f1f604 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Tue, 9 Feb 2021 22:02:46 +0200
Subject: [PATCH 2/4] pam_sss: free env_item when not needed
Signed-off-by: Alexander Bokovoy <aboko...@redhat.com>
---
src/sss_client/pam_sss.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/sss_client/pam_sss.c b/src/sss_client/pam_sss.c
index fa5a9694b8..aa0234c3ae 100644
--- a/src/sss_client/pam_sss.c
+++ b/src/sss_client/pam_sss.c
@@ -996,7 +996,7 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf,
{
int ret;
size_t p=0;
- char *env_item;
+ char *env_item = NULL;
int32_t c;
int32_t type;
int32_t len;
@@ -1020,6 +1020,7 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf,
while(c>0) {
if (buflen < (p+2*sizeof(int32_t))) {
D(("response buffer is too small"));
+ free(env_item);
return PAM_BUF_ERR;
}
@@ -1031,6 +1032,7 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf,
if (buflen < (p + len)) {
D(("response buffer is too small"));
+ free(env_item);
return PAM_BUF_ERR;
}
@@ -1207,6 +1209,9 @@ static int eval_response(pam_handle_t *pamh, size_t buflen, uint8_t *buf,
p += len;
--c;
+
+ free(env_item);
+ env_item = NULL;
}
return PAM_SUCCESS;
@@ -2139,7 +2144,7 @@ static void eval_argv(pam_handle_t *pamh, int argc, const char **argv,
static int prompt_by_config(pam_handle_t *pamh, struct pam_items *pi)
{
size_t c;
- int ret;
+ int ret = PAM_SYSTEM_ERR;
if (pi->pc == NULL || *pi->pc == NULL) {
return PAM_SYSTEM_ERR;
From 2eeb2337d812abc4b88c563078c7256f68a767ea Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Wed, 10 Feb 2021 09:17:08 +0200
Subject: [PATCH 3/4] covscan: initialize ret variable before use
covscan does consider 'ret' unitialized even though
GET_ATTR/GET_ATTR_ARRAY macros have explicit and unconditional
assignment to ret. This is confusing but causing actual failures in
covscan runs.
Signed-off-by: Alexander Bokovoy <aboko...@redhat.com>
---
src/lib/sifp/sss_sifp_attrs.c | 32 ++++++++++++++++----------------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/src/lib/sifp/sss_sifp_attrs.c b/src/lib/sifp/sss_sifp_attrs.c
index 1004252e3b..ed24e84d2d 100644
--- a/src/lib/sifp/sss_sifp_attrs.c
+++ b/src/lib/sifp/sss_sifp_attrs.c
@@ -96,7 +96,7 @@ sss_sifp_find_attr_as_bool(sss_sifp_attr **attrs,
const char *name,
bool *_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_BOOL, boolean, *_value, ret);
return ret;
}
@@ -106,7 +106,7 @@ sss_sifp_find_attr_as_int16(sss_sifp_attr **attrs,
const char *name,
int16_t *_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_INT16, int16, *_value, ret);
return ret;
}
@@ -116,7 +116,7 @@ sss_sifp_find_attr_as_uint16(sss_sifp_attr **attrs,
const char *name,
uint16_t *_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_UINT16, uint16, *_value, ret);
return ret;
}
@@ -126,7 +126,7 @@ sss_sifp_find_attr_as_int32(sss_sifp_attr **attrs,
const char *name,
int32_t *_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_INT32, int32, *_value, ret);
return ret;
}
@@ -136,7 +136,7 @@ sss_sifp_find_attr_as_uint32(sss_sifp_attr **attrs,
const char *name,
uint32_t *_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_UINT32, uint32, *_value, ret);
return ret;
}
@@ -146,7 +146,7 @@ sss_sifp_find_attr_as_int64(sss_sifp_attr **attrs,
const char *name,
int64_t *_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_INT64, int64, *_value, ret);
return ret;
}
@@ -156,7 +156,7 @@ sss_sifp_find_attr_as_uint64(sss_sifp_attr **attrs,
const char *name,
uint64_t *_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_UINT64, uint64, *_value, ret);
return ret;
}
@@ -166,7 +166,7 @@ sss_sifp_find_attr_as_string(sss_sifp_attr **attrs,
const char *name,
const char **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
const char *value = NULL;
GET_ATTR(attrs, name, SSS_SIFP_ATTR_TYPE_STRING, str, value, ret);
@@ -219,7 +219,7 @@ sss_sifp_find_attr_as_bool_array(sss_sifp_attr **attrs,
unsigned int *_num_values,
bool **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR_ARRAY(attrs, name, SSS_SIFP_ATTR_TYPE_BOOL, boolean,
*_num_values, *_value, ret);
return ret;
@@ -231,7 +231,7 @@ sss_sifp_find_attr_as_int16_array(sss_sifp_attr **attrs,
unsigned int *_num_values,
int16_t **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR_ARRAY(attrs, name, SSS_SIFP_ATTR_TYPE_INT16, int16,
*_num_values, *_value, ret);
return ret;
@@ -243,7 +243,7 @@ sss_sifp_find_attr_as_uint16_array(sss_sifp_attr **attrs,
unsigned int *_num_values,
uint16_t **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR_ARRAY(attrs, name, SSS_SIFP_ATTR_TYPE_UINT16, uint16,
*_num_values, *_value, ret);
return ret;
@@ -255,7 +255,7 @@ sss_sifp_find_attr_as_int32_array(sss_sifp_attr **attrs,
unsigned int *_num_values,
int32_t **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR_ARRAY(attrs, name, SSS_SIFP_ATTR_TYPE_INT32, int32,
*_num_values, *_value, ret);
return ret;
@@ -267,7 +267,7 @@ sss_sifp_find_attr_as_uint32_array(sss_sifp_attr **attrs,
unsigned int *_num_values,
uint32_t **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR_ARRAY(attrs, name, SSS_SIFP_ATTR_TYPE_UINT32, uint32,
*_num_values, *_value, ret);
return ret;
@@ -279,7 +279,7 @@ sss_sifp_find_attr_as_int64_array(sss_sifp_attr **attrs,
unsigned int *_num_values,
int64_t **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR_ARRAY(attrs, name, SSS_SIFP_ATTR_TYPE_INT64, int64,
*_num_values, *_value, ret);
return ret;
@@ -291,7 +291,7 @@ sss_sifp_find_attr_as_uint64_array(sss_sifp_attr **attrs,
unsigned int *_num_values,
uint64_t **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
GET_ATTR_ARRAY(attrs, name, SSS_SIFP_ATTR_TYPE_UINT64, uint64,
*_num_values, *_value, ret);
return ret;
@@ -303,7 +303,7 @@ sss_sifp_find_attr_as_string_array(sss_sifp_attr **attrs,
unsigned int *_num_values,
const char * const **_value)
{
- sss_sifp_error ret;
+ sss_sifp_error ret = SSS_SIFP_ATTR_MISSING;
char **value;
GET_ATTR_ARRAY(attrs, name, SSS_SIFP_ATTR_TYPE_STRING, str,
From 273ff9e3ea2fa2838458b47950420f67bc833764 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <aboko...@redhat.com>
Date: Wed, 10 Feb 2021 09:20:45 +0200
Subject: [PATCH 4/4] covscan: symlink() expects non-NULL second argument
There are no checks that talloc()-ed symlink file name is not NULL.
Signed-off-by: Alexander Bokovoy <aboko...@redhat.com>
---
src/sbus/server/sbus_server.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/sbus/server/sbus_server.c b/src/sbus/server/sbus_server.c
index 69efd739b4..4ada9a2599 100644
--- a/src/sbus/server/sbus_server.c
+++ b/src/sbus/server/sbus_server.c
@@ -146,6 +146,10 @@ sbus_server_symlink_create(const char *filename,
{
errno_t ret;
+ if (symlink_filename == NULL) {
+ return ENOMEM;
+ }
+
DEBUG(SSSDBG_TRACE_LIBS, "Symlinking the dbus path %s to a link %s\n",
filename, symlink_filename);
errno = 0;
_______________________________________________
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
