From 2d9e80247d4f3b9dcbd8a146dd053de61d2095e2 Mon Sep 17 00:00:00 2001 From: Iker Pedrosa Date: Jul 01 2021 10:34:16 +0000 Subject: pam_console: fix covscan issues Error: RESOURCE_LEAK (CWE-772): [#def14] Linux-PAM-1.5.1/modules/pam_console/chmod.c:210: alloc_fn: Storage is returned from allocation function "mode_compile". Linux-PAM-1.5.1/modules/pam_console/chmod.c:210: var_assign: Assigning: "changes" = storage returned from "mode_compile(mode, 7U)". Linux-PAM-1.5.1/modules/pam_console/chmod.c:239: leaked_storage: Variable "changes" going out of scope leaks the storage it points to. 237| globfree(&result); 238| 239|-> return (errors); 240| } Error: RESOURCE_LEAK (CWE-772): [#def17] Linux-PAM-1.5.1/modules/pam_console/handlers.c:66: alloc_fn: Storage is returned from allocation function "fopen". Linux-PAM-1.5.1/modules/pam_console/handlers.c:66: var_assign: Assigning: "fh" = storage returned from "fopen(handlers_name, "r")". Linux-PAM-1.5.1/modules/pam_console/handlers.c:74: noescape: Resource "fh" is not freed or pointed-to in "fgets". [Note: The source code implementation of the function has been overridden by a builtin model.] Linux-PAM-1.5.1/modules/pam_console/handlers.c:74: noescape: Resource "fh" is not freed or pointed-to in "fgets". [Note: The source code implementation of the function has been overridden by a builtin model.] Linux-PAM-1.5.1/modules/pam_console/handlers.c:74: noescape: Resource "fh" is not freed or pointed-to in "fgets". [Note: The source code implementation of the function has been overridden by a builtin model.] Linux-PAM-1.5.1/modules/pam_console/handlers.c:148: leaked_storage: Variable "fh" going out of scope leaks the storage it points to. 146| fail_exit: 147| console_free_handlers(first_handler); 148|-> return rv; 149| } 150| Error: COMPILER_WARNING (CWE-686): [#def19] Linux-PAM-1.5.1/modules/pam_console/handlers.c: scope_hint: In function 'execute_handler' Linux-PAM-1.5.1/modules/pam_console/handlers.c:265:29: warning[-Wimplicit-function-declaration]: implicit declaration of function 'setgroups'; did you mean 'getgroups'? 263| _exit(255); 264| if (setgid(pw->pw_gid) == -1 || 265|-> setgroups(0, NULL) == -1 || 266| setuid(pw->pw_uid) == -1) 267| _exit(255); Error: VARARGS (CWE-237): [#def20] Linux-PAM-1.5.1/modules/pam_console/pam_console.c:73: va_init: Initializing va_list "args". Linux-PAM-1.5.1/modules/pam_console/pam_console.c:76: missing_va_end: "va_end" was not called for "args". 74| pam_vsyslog(pamh, err, format, args); 75| closelog(); 76|-> } 77| 78| static void * Error: RESOURCE_LEAK (CWE-772): [#def22] Linux-PAM-1.5.1/modules/pam_console/pam_console.c:148: open_fn: Returning handle opened by "socket". Linux-PAM-1.5.1/modules/pam_console/pam_console.c:148: var_assign: Assigning: "fd" = handle returned from "socket(1, SOCK_STREAM, 0)". Linux-PAM-1.5.1/modules/pam_console/pam_console.c:156: leaked_handle: Handle variable "fd" going out of scope leaks the handle. 154| 155| if (len > sizeof(addr.su.sun_path)) 156|-> return 0; 157| memcpy(addr.su.sun_path, path, len); 158| if (connect(fd, &addr.sa, sizeof(addr.su) - (sizeof(addr.su.sun_path) - len)) == 0) { Signed-off-by: Iker Pedrosa --- diff --git a/pam_console/chmod.c b/pam_console/chmod.c index 777e37f..a674f7d 100644 --- a/pam_console/chmod.c +++ b/pam_console/chmod.c @@ -235,6 +235,7 @@ chmod_files (const char *mode, uid_t user, gid_t group, } globfree(&result); + mode_free(changes); return (errors); } diff --git a/pam_console/handlers.c b/pam_console/handlers.c index ec097c6..1a94578 100644 --- a/pam_console/handlers.c +++ b/pam_console/handlers.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "handlers.h" #include "pam_console.h" @@ -145,6 +146,7 @@ console_parse_handlers (pam_handle_t *pamh, const char *handlers_name) { fail_exit: console_free_handlers(first_handler); + (void) fclose(fh); return rv; } diff --git a/pam_console/pam_console.c b/pam_console/pam_console.c index 5740ea0..10a7f00 100644 --- a/pam_console/pam_console.c +++ b/pam_console/pam_console.c @@ -72,6 +72,7 @@ _pam_log(pam_handle_t *pamh, int err, int debug_p, const char *format, ...) va_start(args, format); pam_vsyslog(pamh, err, format, args); + va_end(args); closelog(); } @@ -152,8 +153,10 @@ try_xsocket(const char *path, size_t len) { memset(&addr, 0, sizeof(addr)); addr.su.sun_family = AF_UNIX; - if (len > sizeof(addr.su.sun_path)) + if (len > sizeof(addr.su.sun_path)) { + close(fd); return 0; + } memcpy(addr.su.sun_path, path, len); if (connect(fd, &addr.sa, sizeof(addr.su) - (sizeof(addr.su.sun_path) - len)) == 0) { close(fd);