Attachment 13810 Details for Bug 11573 – [PATCH 2/2] heimdal: fix CR comments on include/includedir

[patch] [PATCH 2/2] heimdal: fix CR comments on include/includedir

0002-heimdal-fix-CR-comments-on-include-includedir.patch (text/plain), 6.42 KB, created by Evgeny Sinelnikov on 2017-11-25 19:24:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Evgeny Sinelnikov

Created: 2017-11-25 19:24:16 UTC

Size: 6.42 KB

patch

obsolete

>From cf32f7d532952b2bbd661b49bb5f0853d1462294 Mon Sep 17 00:00:00 2001
>From: Evgeny Sinelnikov <sin@altlinux.org>
>Date: Thu, 23 Nov 2017 06:45:58 +0400
>Subject: [PATCH 2/2] heimdal: fix CR comments on include/includedir
>
>---
> source4/heimdal/lib/krb5/config_file.c | 95 +++++++++++++++++++++++++++++-----
> source4/heimdal/lib/krb5/krb5_locl.h   |  7 +++
> 2 files changed, 88 insertions(+), 14 deletions(-)
>
>diff --git a/source4/heimdal/lib/krb5/config_file.c b/source4/heimdal/lib/krb5/config_file.c
>index dfd50cd..4bac296 100644
>--- a/source4/heimdal/lib/krb5/config_file.c
>+++ b/source4/heimdal/lib/krb5/config_file.c
>@@ -337,6 +337,41 @@ parse_plist_config(krb5_context context, const char *path, krb5_config_section *
> 
> #endif
> 
>+static int
>+is_absolute_path(const char *path)
>+{
>+    /*
>+     * An absolute path is one that refers to an explicit object
>+     * without ambiguity.
>+     */
>+#ifdef WIN32
>+    size_t len = strlen(path);
>+
>+    /* UNC path is by definition absolute */
>+    if (len > 2
>+         && ISPATHSEP(path[0])
>+         && ISPATHSEP(path[1]))
>+        return 1;
>+
>+    /* A drive letter path might be absolute */
>+    if (len > 3
>+         && isalpha(path[0])
>+         && path[1] == ':'
>+         && ISPATHSEP(path[2]))
>+        return 1;
>+
>+    /*
>+     * if no drive letter but first char is a path
>+     * separator then the drive letter must be obtained
>+     * from the including file.
>+     */
>+#else
>+    /* UNIX is easy, first char '/' is absolute */
>+    if (ISPATHSEP(path[0]))
>+        return 1;
>+#endif
>+    return 0;
>+}
> 
> /*
>  * Parse the config file `fname', generating the structures into `res'
>@@ -377,6 +412,12 @@ krb5_config_parse_debug (struct fileptr *f,
>             p += sizeof("include");
>             while (isspace(*p))
>                 p++;
>+            if (!is_absolute_path(p)) {
>+                krb5_set_error_message(f->context, EINVAL,
>+                                       "Configuration include path must be "
>+                                       "absolute");
>+                return EINVAL;
>+            }
>             ret = krb5_config_parse_file_multi(f->context, p, res);
> 	    if (ret)
> 		return ret;
>@@ -385,6 +426,12 @@ krb5_config_parse_debug (struct fileptr *f,
>             p += sizeof("includedir");
>             while (isspace(*p))
>                 p++;
>+            if (!is_absolute_path(p)) {
>+                krb5_set_error_message(f->context, EINVAL,
>+                                       "Configuration includedir path must be "
>+                                       "absolute");
>+                return EINVAL;
>+            }
>             ret = krb5_config_parse_dir_multi(f->context, p, res);
> 	    if (ret)
> 		return ret;
>@@ -448,7 +495,14 @@ krb5_config_parse_dir_multi(krb5_context context,
>         int is_valid = 1;
> 
>         while (*p) {
>-            if (!isalpha(*p) && *p != '_' && *p != '-' &&
>+            /*
>+             * Here be dragons.  The call to krb5_config_parse_file_multi()
>+             * below expands path tokens.  Because of the limitations here
>+             * on file naming, we can't have path tokens in the file name,
>+             * so we're safe.  Anyone changing this if condition here should
>+             * be aware.
>+             */
>+            if (!isalnum(*p) && *p != '_' && *p != '-' &&
>                 strcmp(p, ".conf") != 0) {
>                 is_valid = 0;
>                 break;
>@@ -459,13 +513,17 @@ krb5_config_parse_dir_multi(krb5_context context,
>             continue;
> 
>         if (asprintf(&path, "%s/%s", dname, entry->d_name) == -1 ||
>-            path == NULL)
>+            path == NULL) {
>+            (void) closedir(d);
>             return krb5_enomem(context);
>+        }
>         ret = krb5_config_parse_file_multi(context, path, res);
>         free(path);
>-        if (ret == ENOMEM)
>+        if (ret == ENOMEM) {
>+            (void) closedir(d);
>             return krb5_enomem(context);;
>-        /* Ignore malformed config files */
>+        }
>+        /* Ignore malformed config files so we don't lock out admins, etc... */
>     }
>     (void) closedir(d);
>     return 0;
>@@ -494,6 +552,7 @@ krb5_config_parse_file_multi (krb5_context context,
>     unsigned lineno = 0;
>     krb5_error_code ret;
>     struct fileptr f;
>+    struct stat st;
> 
>     if (context->config_include_depth > 5) {
>         krb5_warnx(context, "Maximum config file include depth reached; "
>@@ -550,14 +609,13 @@ krb5_config_parse_file_multi (krb5_context context,
>     }
> 
>     if (is_plist_file(fname)) {
>+        context->config_include_depth--;
> #ifdef __APPLE__
> 	ret = parse_plist_config(context, fname, res);
>-        context->config_include_depth--;
> 	if (ret) {
> 	    krb5_set_error_message(context, ret,
> 				   "Failed to parse plist %s", fname);
>-	    if (newfname)
>-		free(newfname);
>+            free(newfname);
> 	    return ret;
> 	}
> #else
>@@ -585,24 +643,33 @@ krb5_config_parse_file_multi (krb5_context context,
>         f.context = context;
> 	f.f = fopen(fname, "r");
> 	f.s = NULL;
>-	if(f.f == NULL) {
>+	if (f.f == NULL || fstat(fileno(f.f), &st) == -1) {
>+            if (f.f != NULL)
>+                (void) fclose(f.f);
>             context->config_include_depth--;
> 	    ret = errno;
>-	    krb5_set_error_message (context, ret, "open %s: %s",
>-				    fname, strerror(ret));
>-	    if (newfname)
>-		free(newfname);
>+	    krb5_set_error_message(context, ret, "open or stat %s: %s",
>+				   fname, strerror(ret));
>+           free(newfname);
> 	    return ret;
> 	}
> 
>+        if (!S_ISREG(st.st_mode)) {
>+            (void) fclose(f.f);
>+            context->config_include_depth--;
>+            free(newfname);
>+	    krb5_set_error_message(context, EISDIR, "not a regular file %s: %s",
>+				   fname, strerror(EISDIR));
>+	    return EISDIR;
>+        }
>+
> 	ret = krb5_config_parse_debug (&f, res, &lineno, &str);
>         context->config_include_depth--;
> 	fclose(f.f);
> 	if (ret) {
> 	    krb5_set_error_message (context, ret, "%s:%u: %s",
> 				    fname, lineno, str);
>-	    if (newfname)
>-		free(newfname);
>+            free(newfname);
> 	    return ret;
> 	}
>     }
>diff --git a/source4/heimdal/lib/krb5/krb5_locl.h b/source4/heimdal/lib/krb5/krb5_locl.h
>index f9c40e3..ab0cf87 100644
>--- a/source4/heimdal/lib/krb5/krb5_locl.h
>+++ b/source4/heimdal/lib/krb5/krb5_locl.h
>@@ -358,4 +358,11 @@ enum krb5_pk_type {
> 
> #endif /* PKINIT */
> 
>+#define ISTILDE(x) (x == '~')
>+#ifdef _WIN32
>+# define ISPATHSEP(x) (x == '/' || x =='\\')
>+#else
>+# define ISPATHSEP(x) (x == '/')
>+#endif
>+
> #endif /* __KRB5_LOCL_H__ */
>-- 
>2.10.2
>

Actions: View

Attachments on bug 11573: 11593 | 13809 | 13810 | 13819 | 13820 | 14239 | 14246