From 4b227020c964b112b3882abf1df69a3f25faa07d Mon Sep 17 00:00:00 2001 From: Evgeny Sinelnikov Date: Thu, 23 Nov 2017 06:45:58 +0400 Subject: [PATCH 2/2] heimdal: fix CR comments on include/includedir Backport includedir support from Heimdal based on next commits: - partly cherry-picked from (for lib/krb5/krb5_locl.h only) https://github.com/heimdal/heimdal/commit/31a00d664715fb4c7b90c1617a3b4c2580282d7b - modified from (fix CR comments on include/includedir) https://github.com/heimdal/heimdal/commit/0259f1c44927ab8f5906212804693dec48c9a04a --- 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