krb5 commit: Improve code hygiene of kdb5_util dump helpers

Greg Hudson <[email protected]>
Newsgroups gmane.comp.encryption.kerberos.cvs
Message-ID <[email protected]>
https://github.com/krb5/krb5/commit/e91359aead19f432e5fbc99054a793e983f58328
commit e91359aead19f432e5fbc99054a793e983f58328
Author: Greg Hudson <[email protected]>
Date:   Thu Oct 25 12:55:50 2018 -0400

    Improve code hygiene of kdb5_util dump helpers
    
    kdb5_util dump can very briefly leak a file handle if the ok file
    cannot be locked, or if the existing dump file cannot be read.  Add a
    cleanup handler to prep_ok_file() and use proper output parameter
    handling.  Change current_dump_sno_in_ulog() to close its file handle
    before checking the result of fgets().  Reported by Bean Zhang.

 src/kadmin/dbutil/dump.c |   45 ++++++++++++++++++++++++++++-----------------
 1 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/kadmin/dbutil/dump.c b/src/kadmin/dbutil/dump.c
index 86e046c..c9574c6 100644
--- a/src/kadmin/dbutil/dump.c
+++ b/src/kadmin/dbutil/dump.c
@@ -181,34 +181,44 @@ finish_ofile(char *ofile, char **tmpname)
 }
 
 /* Create the .dump_ok file. */
-static int
-prep_ok_file(krb5_context context, char *file_name, int *fd)
+static krb5_boolean
+prep_ok_file(krb5_context context, char *file_name, int *fd_out)
 {
     static char ok[] = ".dump_ok";
     krb5_error_code retval;
-    char *file_ok;
+    char *file_ok = NULL;
+    int fd = -1;
+    krb5_boolean success = FALSE;
+
+    *fd_out = -1;
 
     if (asprintf(&file_ok, "%s%s", file_name, ok) < 0) {
         com_err(progname, ENOMEM, _("while allocating dump_ok filename"));
-        exit_status++;
-        return 0;
+        goto cleanup;
     }
 
-    *fd = open(file_ok, O_WRONLY | O_CREAT | O_TRUNC, 0600);
-    if (*fd == -1) {
+    fd = open(file_ok, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+    if (fd == -1) {
         com_err(progname, errno, _("while creating 'ok' file, '%s'"), file_ok);
-        exit_status++;
-        free(file_ok);
-        return 0;
+        goto cleanup;
     }
-    retval = krb5_lock_file(context, *fd, KRB5_LOCKMODE_EXCLUSIVE);
+    retval = krb5_lock_file(context, fd, KRB5_LOCKMODE_EXCLUSIVE);
     if (retval) {
         com_err(progname, retval, _("while locking 'ok' file, '%s'"), file_ok);
-        free(file_ok);
-        return 0;
+        goto cleanup;
     }
+
+    *fd_out = fd;
+    fd = -1;
+    success = TRUE;
+
+cleanup:
     free(file_ok);
-    return 1;
+    if (fd != -1)
+        close(fd);
+    if (!success)
+        exit_status++;
+    return success;
 }
 
 /*
@@ -1227,16 +1237,17 @@ current_dump_sno_in_ulog(krb5_context context, const char *ifile)
     update_status_t status;
     dump_version *junk;
     kdb_last_t last;
-    char buf[BUFSIZ];
+    char buf[BUFSIZ], *r;
     FILE *f;
 
     f = fopen(ifile, "r");
     if (f == NULL)
         return 0;              /* aliasing other errors to ENOENT here is OK */
 
-    if (fgets(buf, sizeof(buf), f) == NULL)
-        return errno ? -1 : 0;
+    r = fgets(buf, sizeof(buf), f);
     fclose(f);
+    if (r == NULL)
+        return errno ? -1 : 0;
 
     if (!parse_iprop_header(buf, &junk, &last))
         return 0;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.