From 111b8b4d62a4fe192c075e6f6bfacb408e6074b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 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 Reviewed-by: Sumit Bose --- 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