From df5dafe049aa26e8834d83dd6274b28dc32a217c Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sat, 11 Jul 2015 13:00:13 +0200 Subject: [PATCH 1/2] Clear passwords on __gr_dup/__pw_dup errors. The functions __gr_dup and __pw_dup do not explicitly zero the memory which hold the passwords after free. The gr_free and pw_free functions do this explicitly. To guarantee same behaviour, it's possible to call these *_free functions directly from __*_dup, because the memory is initialized with zeros at the beginning. Calling free(NULL) has no negative effect and can be considered safe these days. --- lib/groupmem.c | 17 ++++------------- lib/pwmem.c | 20 +++++--------------- 2 files changed, 9 insertions(+), 28 deletions(-) diff --git a/lib/groupmem.c b/lib/groupmem.c index e69c3107..1fd1c135 100644 --- a/lib/groupmem.c +++ b/lib/groupmem.c @@ -55,15 +55,14 @@ gr->gr_name = strdup (grent->gr_name); /*@=mustfreeonly@*/ if (NULL == gr->gr_name) { - free(gr); + gr_free(gr); return NULL; } /*@-mustfreeonly@*/ gr->gr_passwd = strdup (grent->gr_passwd); /*@=mustfreeonly@*/ if (NULL == gr->gr_passwd) { - free(gr->gr_name); - free(gr); + gr_free(gr); return NULL; } @@ -73,21 +72,13 @@ gr->gr_mem = (char **) malloc ((i + 1) * sizeof (char *)); /*@=mustfreeonly@*/ if (NULL == gr->gr_mem) { - free(gr->gr_passwd); - free(gr->gr_name); - free(gr); + gr_free(gr); return NULL; } for (i = 0; grent->gr_mem[i]; i++) { gr->gr_mem[i] = strdup (grent->gr_mem[i]); if (NULL == gr->gr_mem[i]) { - int j; - for (j=0; jgr_mem[j]); - free(gr->gr_mem); - free(gr->gr_passwd); - free(gr->gr_name); - free(gr); + gr_free(gr); return NULL; } } diff --git a/lib/pwmem.c b/lib/pwmem.c index 7013e8a3..17d2eb21 100644 --- a/lib/pwmem.c +++ b/lib/pwmem.c @@ -56,45 +56,35 @@ pw->pw_name = strdup (pwent->pw_name); /*@=mustfreeonly@*/ if (NULL == pw->pw_name) { - free(pw); + pw_free(pw); return NULL; } /*@-mustfreeonly@*/ pw->pw_passwd = strdup (pwent->pw_passwd); /*@=mustfreeonly@*/ if (NULL == pw->pw_passwd) { - free(pw->pw_name); - free(pw); + pw_free(pw); return NULL; } /*@-mustfreeonly@*/ pw->pw_gecos = strdup (pwent->pw_gecos); /*@=mustfreeonly@*/ if (NULL == pw->pw_gecos) { - free(pw->pw_passwd); - free(pw->pw_name); - free(pw); + pw_free(pw); return NULL; } /*@-mustfreeonly@*/ pw->pw_dir = strdup (pwent->pw_dir); /*@=mustfreeonly@*/ if (NULL == pw->pw_dir) { - free(pw->pw_gecos); - free(pw->pw_passwd); - free(pw->pw_name); - free(pw); + pw_free(pw); return NULL; } /*@-mustfreeonly@*/ pw->pw_shell = strdup (pwent->pw_shell); /*@=mustfreeonly@*/ if (NULL == pw->pw_shell) { - free(pw->pw_dir); - free(pw->pw_gecos); - free(pw->pw_passwd); - free(pw->pw_name); - free(pw); + pw_free(pw); return NULL; } From c17f5ec460132ca0b8f066d1c75672cbcce6a0fa Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Sun, 12 Jul 2015 14:30:32 +0200 Subject: [PATCH 2/2] Free memory on error path When multiple entries with the same name are encountered, nentry is not properly freed, which results in a memory leak. --- lib/commonio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/commonio.c b/lib/commonio.c index cc536bf1..8ff0e07d 100644 --- a/lib/commonio.c +++ b/lib/commonio.c @@ -1081,6 +1081,7 @@ int commonio_update (struct commonio_db *db, const void *eptr) if (NULL != p) { if (next_entry_by_name (db, p->next, db->ops->getname (eptr)) != NULL) { fprintf (stderr, _("Multiple entries named '%s' in %s. Please fix this with pwck or grpck.\n"), db->ops->getname (eptr), db->filename); + db->ops->free (nentry); return 0; } db->ops->free (p->eptr);