From 111b8b4d62a4fe192c075e6f6bfacb408e6074b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina@redhat.com>
Date: Tue, 12 Jan 2021 13:50:11 +0100
Subject: [PATCH 39/39] pam_sss_gssapi: fix coverity issues
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
```
1. Defect type: RESOURCE_LEAK
7. sssd-2.4.0/src/sss_client/pam_sss_gss.c:556: leaked_storage: Variable "username" going out of scope leaks the storage it points to.
Expand
2. Defect type: RESOURCE_LEAK
3. sssd-2.4.0/src/sss_client/pam_sss_gss.c:321: leaked_storage: Variable "reply" going out of scope leaks the storage it points to.
Expand
3. Defect type: RESOURCE_LEAK
7. sssd-2.4.0/src/sss_client/pam_sss_gss.c:260: leaked_storage: Variable "username" going out of scope leaks the storage it points to.
Expand
4. Defect type: RESOURCE_LEAK
6. sssd-2.4.0/src/sss_client/pam_sss_gss.c:260: leaked_storage: Variable "upn" going out of scope leaks the storage it points to.
Expand
5. Defect type: RESOURCE_LEAK
7. sssd-2.4.0/src/sss_client/pam_sss_gss.c:260: leaked_storage: Variable "target" going out of scope leaks the storage it points to.
Expand
6. Defect type: RESOURCE_LEAK
7. sssd-2.4.0/src/sss_client/pam_sss_gss.c:260: leaked_storage: Variable "domain" going out of scope leaks the storage it points to.
1. Defect type: CLANG_WARNING
1. sssd-2.4.0/src/sss_client/pam_sss_gss.c:260:16: warning[unix.Malloc]: Potential leak of memory pointed to by 'username'
Expand
2. Defect type: CLANG_WARNING
1. sssd-2.4.0/src/sss_client/pam_sss_gss.c:260:16: warning[unix.Malloc]: Potential leak of memory pointed to by 'upn'
Expand
3. Defect type: CLANG_WARNING
1. sssd-2.4.0/src/sss_client/pam_sss_gss.c:260:16: warning[unix.Malloc]: Potential leak of memory pointed to by 'target'
Expand
4. Defect type: CLANG_WARNING
1. sssd-2.4.0/src/sss_client/pam_sss_gss.c:260:16: warning[unix.Malloc]: Potential leak of memory pointed to by 'domain'
```
Also fix compilation warning
```
../src/sss_client/pam_sss_gss.c:339:5: warning: ‘reply’ may be used uninitialized in this function [-Wmaybe-uninitialized]
339 | free(reply);
| ^~~~~~~~~~~
../src/sss_client/pam_sss_gss.c:328:14: note: ‘reply’ was declared here
328 | uint8_t *reply;
| ^~~~~
../src/sss_client/pam_sss_gss.c:270:11: warning: ‘reply_len’ may be used uninitialized in this function [-Wmaybe-uninitialized]
270 | upn = malloc(reply_len * sizeof(char));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/sss_client/pam_sss_gss.c:327:12: note: ‘reply_len’ was declared here
327 | size_t reply_len;
| ^~~~~~~~~
```
Reviewed-by: Alexey Tikhonov <atikhono@redhat.com>
Reviewed-by: Sumit Bose <sbose@redhat.com>
---
src/sss_client/pam_sss_gss.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/sss_client/pam_sss_gss.c b/src/sss_client/pam_sss_gss.c
index cd38db7da..51be36ece 100644
--- a/src/sss_client/pam_sss_gss.c
+++ b/src/sss_client/pam_sss_gss.c
@@ -195,6 +195,8 @@ static errno_t sssd_gssapi_init_send(pam_handle_t *pamh,
struct sss_cli_req_data req_data;
size_t service_len;
size_t user_len;
+ size_t reply_len;
+ uint8_t *reply = NULL;
uint8_t *data;
errno_t ret;
int ret_errno;
@@ -217,7 +219,7 @@ static errno_t sssd_gssapi_init_send(pam_handle_t *pamh,
req_data.data = data;
- ret = sss_pam_make_request(SSS_GSSAPI_INIT, &req_data, _reply, _reply_len,
+ ret = sss_pam_make_request(SSS_GSSAPI_INIT, &req_data, &reply, &reply_len,
&ret_errno);
free(data);
if (ret != PAM_SUCCESS) {
@@ -233,6 +235,16 @@ static errno_t sssd_gssapi_init_send(pam_handle_t *pamh,
return (ret_errno != EOK) ? ret_errno : EIO;
}
+ if (ret_errno == EOK) {
+ *_reply = reply;
+ *_reply_len = reply_len;
+ } else {
+ /* We got PAM_SUCCESS therefore the communication with SSSD was
+ * successful and we have received a reply buffer. We just don't care
+ * about it, we are only interested in the error code. */
+ free(reply);
+ }
+
return ret_errno;
}
@@ -257,7 +269,8 @@ static errno_t sssd_gssapi_init_recv(uint8_t *reply,
target = malloc(reply_len * sizeof(char));
upn = malloc(reply_len * sizeof(char));
if (username == NULL || domain == NULL || target == NULL || upn == NULL) {
- return ENOMEM;
+ ret = ENOMEM;
+ goto done;
}
buf = (const char*)reply;
@@ -311,8 +324,8 @@ static errno_t sssd_gssapi_init(pam_handle_t *pamh,
char **_target,
char **_upn)
{
- size_t reply_len;
- uint8_t *reply;
+ size_t reply_len = 0;
+ uint8_t *reply = NULL;
errno_t ret;
ret = sssd_gssapi_init_send(pamh, pam_service, pam_user, &reply,
@@ -549,6 +562,7 @@ int pam_sm_authenticate(pam_handle_t *pamh,
done:
sss_pam_close_fd();
+ free(username);
free(domain);
free(target);
free(upn);
--
2.21.3