From 1e7799e8b84491cadb99cc306749316beec8a339 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Tue, 29 Jan 2013 12:15:18 -0800 Subject: [PATCH] Implement config key validation rules This is a new implementation of core git's config key checking rules that prevents non-alphanumeric characters (and '-') for the top-level section and key names inside of config files. This also validates the target section name when renaming sections. --- src/config.c | 87 ++++++++++++++++++++++++----------------------- src/config_file.c | 70 +++++++++++++++++++++++++++----------- src/config_file.h | 2 ++ 3 files changed, 98 insertions(+), 61 deletions(-) diff --git a/src/config.c b/src/config.c index 84f3ba0f9..ce105089e 100644 --- a/src/config.c +++ b/src/config.c @@ -11,6 +11,7 @@ #include "git2/config.h" #include "vector.h" #include "buf_text.h" +#include "config_file.h" #if GIT_WIN32 # include #endif @@ -758,42 +759,36 @@ fail_parse: return -1; } -struct rename_data -{ +struct rename_data { git_config *config; - const char *old_name; - const char *new_name; + git_buf *name; + size_t old_len; + int actual_error; }; static int rename_config_entries_cb( const git_config_entry *entry, void *payload) { + int error = 0; struct rename_data *data = (struct rename_data *)payload; + size_t base_len = git_buf_len(data->name); - if (data->new_name != NULL) { - git_buf name = GIT_BUF_INIT; - int error; - - if (git_buf_printf( - &name, - "%s.%s", - data->new_name, - entry->name + strlen(data->old_name) + 1) < 0) - return -1; - + if (base_len > 0 && + !(error = git_buf_puts(data->name, entry->name + data->old_len))) + { error = git_config_set_string( - data->config, - git_buf_cstr(&name), - entry->value); + data->config, git_buf_cstr(data->name), entry->value); - git_buf_free(&name); - - if (error) - return error; + git_buf_truncate(data->name, base_len); } - return git_config_delete_entry(data->config, entry->name); + if (!error) + error = git_config_delete_entry(data->config, entry->name); + + data->actual_error = error; /* preserve actual error code */ + + return error; } int git_config_rename_section( @@ -802,36 +797,44 @@ int git_config_rename_section( const char *new_section_name) { git_config *config; - git_buf pattern = GIT_BUF_INIT; - int error = -1; + git_buf pattern = GIT_BUF_INIT, replace = GIT_BUF_INIT; + int error = 0; struct rename_data data; - git_buf_text_puts_escape_regex(&pattern, old_section_name); - git_buf_puts(&pattern, "\\..+"); - if (git_buf_oom(&pattern)) + git_buf_text_puts_escape_regex(&pattern, old_section_name); + + if ((error = git_buf_puts(&pattern, "\\..+")) < 0) goto cleanup; - if (git_repository_config__weakptr(&config, repo) < 0) + if ((error = git_repository_config__weakptr(&config, repo)) < 0) goto cleanup; - data.config = config; - data.old_name = old_section_name; - data.new_name = new_section_name; + data.config = config; + data.name = &replace; + data.old_len = strlen(old_section_name) + 1; + data.actual_error = 0; - if ((error = git_config_foreach_match( - config, - git_buf_cstr(&pattern), - rename_config_entries_cb, &data)) < 0) { - giterr_set(GITERR_CONFIG, - "Cannot rename config section '%s' to '%s'", - old_section_name, - new_section_name); - goto cleanup; + if ((error = git_buf_join(&replace, '.', new_section_name, "")) < 0) + goto cleanup; + + if (new_section_name != NULL && + (error = git_config_file_normalize_section( + replace.ptr, strchr(replace.ptr, '.'))) < 0) + { + giterr_set( + GITERR_CONFIG, "Invalid config section '%s'", new_section_name); + goto cleanup; } - error = 0; + error = git_config_foreach_match( + config, git_buf_cstr(&pattern), rename_config_entries_cb, &data); + + if (error == GIT_EUSER) + error = data.actual_error; cleanup: git_buf_free(&pattern); + git_buf_free(&replace); + return error; } diff --git a/src/config_file.c b/src/config_file.c index 2b1be05bf..8b51ab21b 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -105,6 +105,29 @@ static void cvar_free(cvar_t *var) git__free(var); } +int git_config_file_normalize_section(char *start, char *end) +{ + char *scan; + + if (start == end) + return GIT_EINVALIDSPEC; + + /* Validate and downcase range */ + for (scan = start; *scan; ++scan) { + if (end && scan >= end) + break; + if (isalnum(*scan)) + *scan = tolower(*scan); + else if (*scan != '-' || scan == start) + return GIT_EINVALIDSPEC; + } + + if (scan == start) + return GIT_EINVALIDSPEC; + + return 0; +} + /* Take something the user gave us and make it nice for our hash function */ static int normalize_name(const char *in, char **out) { @@ -118,19 +141,26 @@ static int normalize_name(const char *in, char **out) fdot = strchr(name, '.'); ldot = strrchr(name, '.'); - if (fdot == NULL || ldot == NULL) { - git__free(name); - giterr_set(GITERR_CONFIG, - "Invalid variable name: '%s'", in); - return -1; - } + if (fdot == NULL || fdot == name || ldot == NULL || !ldot[1]) + goto invalid; - /* Downcase up to the first dot and after the last one */ - git__strntolower(name, fdot - name); - git__strtolower(ldot); + /* Validate and downcase up to first dot and after last dot */ + if (git_config_file_normalize_section(name, fdot) < 0 || + git_config_file_normalize_section(ldot + 1, NULL) < 0) + goto invalid; + + /* If there is a middle range, make sure it doesn't have newlines */ + while (fdot < ldot) + if (*fdot++ == '\n') + goto invalid; *out = name; return 0; + +invalid: + git__free(name); + giterr_set(GITERR_CONFIG, "Invalid config item name '%s'", in); + return GIT_EINVALIDSPEC; } static void free_vars(git_strmap *values) @@ -271,8 +301,8 @@ static int config_set(git_config_backend *cfg, const char *name, const char *val khiter_t pos; int rval, ret; - if (normalize_name(name, &key) < 0) - return -1; + if ((rval = normalize_name(name, &key)) < 0) + return rval; /* * Try to find it in the existing values and update it if it @@ -352,9 +382,10 @@ static int config_get(const git_config_backend *cfg, const char *name, const git diskfile_backend *b = (diskfile_backend *)cfg; char *key; khiter_t pos; + int error; - if (normalize_name(name, &key) < 0) - return -1; + if ((error = normalize_name(name, &key)) < 0) + return error; pos = git_strmap_lookup_index(b->values, key); git__free(key); @@ -379,9 +410,10 @@ static int config_get_multivar( diskfile_backend *b = (diskfile_backend *)cfg; char *key; khiter_t pos; + int error; - if (normalize_name(name, &key) < 0) - return -1; + if ((error = normalize_name(name, &key)) < 0) + return error; pos = git_strmap_lookup_index(b->values, key); git__free(key); @@ -444,8 +476,8 @@ static int config_set_multivar( assert(regexp); - if (normalize_name(name, &key) < 0) - return -1; + if ((result = normalize_name(name, &key)) < 0) + return result; pos = git_strmap_lookup_index(b->values, key); if (!git_strmap_valid_index(b->values, pos)) { @@ -515,8 +547,8 @@ static int config_delete(git_config_backend *cfg, const char *name) int result; khiter_t pos; - if (normalize_name(name, &key) < 0) - return -1; + if ((result = normalize_name(name, &key)) < 0) + return result; pos = git_strmap_lookup_index(b->values, key); git__free(key); diff --git a/src/config_file.h b/src/config_file.h index cf1a59036..7445859c4 100644 --- a/src/config_file.h +++ b/src/config_file.h @@ -54,5 +54,7 @@ GIT_INLINE(int) git_config_file_foreach_match( return cfg->foreach(cfg, regexp, fn, data); } +extern int git_config_file_normalize_section(char *start, char *end); + #endif