From 8281c82e324b57b3a4b520afad26b43ce128d521 Mon Sep 17 00:00:00 2001 From: Iker Pedrosa Date: Fri, 11 Jun 2021 11:50:49 +0200 Subject: [PATCH] usermod.c: fix covscan RESOURCE_LEAK Error: RESOURCE_LEAK (CWE-772): [#def31] shadow-4.8.1/src/usermod.c:813: alloc_fn: Storage is returned from allocation function "__gr_dup". shadow-4.8.1/src/usermod.c:813: var_assign: Assigning: "ngrp" = storage returned from "__gr_dup(grp)". shadow-4.8.1/src/usermod.c:892: leaked_storage: Variable "ngrp" going out of scope leaks the storage it points to. 890| } 891| } 892|-> } 893| 894| #ifdef SHADOWGRP Error: RESOURCE_LEAK (CWE-772): [#def32] shadow-4.8.1/src/usermod.c:933: alloc_fn: Storage is returned from allocation function "__sgr_dup". shadow-4.8.1/src/usermod.c:933: var_assign: Assigning: "nsgrp" = storage returned from "__sgr_dup(sgrp)". shadow-4.8.1/src/usermod.c:1031: leaked_storage: Variable "nsgrp" going out of scope leaks the storage it points to. 1029| } 1030| } 1031|-> } 1032| #endif /* SHADOWGRP */ 1033| Error: RESOURCE_LEAK (CWE-772): [#def34] shadow-4.8.1/src/usermod.c:1161: alloc_fn: Storage is returned from allocation function "getgr_nam_gid". shadow-4.8.1/src/usermod.c:1161: var_assign: Assigning: "grp" = storage returned from "getgr_nam_gid(optarg)". shadow-4.8.1/src/usermod.c:1495: leaked_storage: Variable "grp" going out of scope leaks the storage it points to. 1493| } 1494| #endif /* ENABLE_SUBIDS */ 1495|-> } 1496| 1497| /* Error: RESOURCE_LEAK (CWE-772): [#def35] shadow-4.8.1/src/usermod.c:1991: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.] shadow-4.8.1/src/usermod.c:1991: var_assign: Assigning: "fd" = handle returned from "open("/var/log/lastlog", 2)". shadow-4.8.1/src/usermod.c:2000: noescape: Resource "fd" is not freed or pointed-to in "lseek". shadow-4.8.1/src/usermod.c:2000: noescape: Resource "fd" is not freed or pointed-to in "read". [Note: The source code implementation of the function has been overridden by a builtin model.] shadow-4.8.1/src/usermod.c:2003: noescape: Resource "fd" is not freed or pointed-to in "lseek". shadow-4.8.1/src/usermod.c:2032: leaked_handle: Handle variable "fd" going out of scope leaks the handle. 2030| } 2031| } 2032|-> } 2033| 2034| /* Error: RESOURCE_LEAK (CWE-772): [#def36] shadow-4.8.1/src/usermod.c:2052: open_fn: Returning handle opened by "open". [Note: The source code implementation of the function has been overridden by a user model.] shadow-4.8.1/src/usermod.c:2052: var_assign: Assigning: "fd" = handle returned from "open("/var/log/faillog", 2)". shadow-4.8.1/src/usermod.c:2061: noescape: Resource "fd" is not freed or pointed-to in "lseek". shadow-4.8.1/src/usermod.c:2061: noescape: Resource "fd" is not freed or pointed-to in "read". [Note: The source code implementation of the function has been overridden by a builtin model.] shadow-4.8.1/src/usermod.c:2064: noescape: Resource "fd" is not freed or pointed-to in "lseek". shadow-4.8.1/src/usermod.c:2092: leaked_handle: Handle variable "fd" going out of scope leaks the handle. 2090| } 2091| } 2092|-> } 2093| 2094| #ifndef NO_MOVE_MAILBOX --- src/usermod.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/usermod.c b/src/usermod.c index 7870ba57..03bb9b9d 100644 --- a/src/usermod.c +++ b/src/usermod.c @@ -871,6 +871,8 @@ static void update_group (void) SYSLOG ((LOG_WARN, "failed to prepare the new %s entry '%s'", gr_dbname (), ngrp->gr_name)); fail_exit (E_GRP_UPDATE); } + + gr_free(ngrp); } } @@ -1006,6 +1008,8 @@ static void update_gshadow (void) sgr_dbname (), nsgrp->sg_name)); fail_exit (E_GRP_UPDATE); } + + free (nsgrp); } } #endif /* SHADOWGRP */ @@ -1152,6 +1156,7 @@ static void process_flags (int argc, char **argv) } user_newgid = grp->gr_gid; gflg = true; + gr_free (grp); break; case 'G': if (get_groups (optarg) != 0) { @@ -1995,8 +2000,7 @@ static void update_lastlog (void) /* Copy the old entry to its new location */ if ( (lseek (fd, off_newuid, SEEK_SET) != off_newuid) || (write (fd, &ll, sizeof ll) != (ssize_t) sizeof ll) - || (fsync (fd) != 0) - || (close (fd) != 0)) { + || (fsync (fd) != 0)) { fprintf (stderr, _("%s: failed to copy the lastlog entry of user %lu to user %lu: %s\n"), Prog, (unsigned long) user_id, (unsigned long) user_newid, strerror (errno)); @@ -2012,16 +2016,15 @@ static void update_lastlog (void) memzero (&ll, sizeof (ll)); if ( (lseek (fd, off_newuid, SEEK_SET) != off_newuid) || (write (fd, &ll, sizeof ll) != (ssize_t) sizeof ll) - || (fsync (fd) != 0) - || (close (fd) != 0)) { + || (fsync (fd) != 0)) { fprintf (stderr, _("%s: failed to copy the lastlog entry of user %lu to user %lu: %s\n"), Prog, (unsigned long) user_id, (unsigned long) user_newid, strerror (errno)); } - } else { - (void) close (fd); } } + + (void) close (fd); } /* @@ -2056,8 +2059,7 @@ static void update_faillog (void) /* Copy the old entry to its new location */ if ( (lseek (fd, off_newuid, SEEK_SET) != off_newuid) || (write (fd, &fl, sizeof fl) != (ssize_t) sizeof fl) - || (fsync (fd) != 0) - || (close (fd) != 0)) { + || (fsync (fd) != 0)) { fprintf (stderr, _("%s: failed to copy the faillog entry of user %lu to user %lu: %s\n"), Prog, (unsigned long) user_id, (unsigned long) user_newid, strerror (errno)); @@ -2072,16 +2074,15 @@ static void update_faillog (void) /* Reset the new uid's faillog entry */ memzero (&fl, sizeof (fl)); if ( (lseek (fd, off_newuid, SEEK_SET) != off_newuid) - || (write (fd, &fl, sizeof fl) != (ssize_t) sizeof fl) - || (close (fd) != 0)) { + || (write (fd, &fl, sizeof fl) != (ssize_t) sizeof fl)) { fprintf (stderr, _("%s: failed to copy the faillog entry of user %lu to user %lu: %s\n"), Prog, (unsigned long) user_id, (unsigned long) user_newid, strerror (errno)); } - } else { - (void) close (fd); } } + + (void) close (fd); } #ifndef NO_MOVE_MAILBOX