config: remove unneeded struct field
As well as receiving the config key and value, config callbacks also receive a "struct key_value_info" containing information about the source of the key-value pair. Accessing the "path" field of this struct from a callback passed to repo_config() results in a use-after-free. This happens because repo_config() first populates a configset by calling config_with_options() and then iterates over the configset with the callback passed by the caller. When the configset is constructed it takes a shallow copy of the "struct key_value_info" for each config setting. This leads to the use-after-free as the "path" member is freed before config_with_options() returns. We could fix this by interning the "path" field as we do for the "filename" field but the "path" field is not actually needed. It is populated with a copy of the "path" field from "struct config_source". That field was added ind14d42440d(config: disallow relative include paths from blobs, 2014-02-19) to distinguish between relative include directives in files and those in blobs. However, since1b8132d99d(i18n: config: unfold error messages marked for translation, 2016-07-28) we can differentiate these by looking at the "origin_type" field in "struct key_value_info". So let's remove the "path" members from "struct config_source" and "struct key_value_info" and instead use a combination of the "filename" and "origin_type" fields to determine the absolute path of relative includes. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
committed by
Junio C Hamano
parent
16bd9f20a4
commit
14d7583beb
28
config.c
28
config.c
@@ -56,7 +56,6 @@ struct config_source {
|
|||||||
} u;
|
} u;
|
||||||
enum config_origin_type origin_type;
|
enum config_origin_type origin_type;
|
||||||
const char *name;
|
const char *name;
|
||||||
const char *path;
|
|
||||||
enum config_error_action default_error_action;
|
enum config_error_action default_error_action;
|
||||||
int linenr;
|
int linenr;
|
||||||
int eof;
|
int eof;
|
||||||
@@ -173,14 +172,14 @@ static int handle_path_include(const struct key_value_info *kvi,
|
|||||||
if (!is_absolute_path(path)) {
|
if (!is_absolute_path(path)) {
|
||||||
char *slash;
|
char *slash;
|
||||||
|
|
||||||
if (!kvi || !kvi->path) {
|
if (!kvi || kvi->origin_type != CONFIG_ORIGIN_FILE) {
|
||||||
ret = error(_("relative config includes must come from files"));
|
ret = error(_("relative config includes must come from files"));
|
||||||
goto cleanup;
|
goto cleanup;
|
||||||
}
|
}
|
||||||
|
|
||||||
slash = find_last_dir_sep(kvi->path);
|
slash = find_last_dir_sep(kvi->filename);
|
||||||
if (slash)
|
if (slash)
|
||||||
strbuf_add(&buf, kvi->path, slash - kvi->path + 1);
|
strbuf_add(&buf, kvi->filename, slash - kvi->filename + 1);
|
||||||
strbuf_addstr(&buf, path);
|
strbuf_addstr(&buf, path);
|
||||||
path = buf.buf;
|
path = buf.buf;
|
||||||
}
|
}
|
||||||
@@ -224,11 +223,11 @@ static int prepare_include_condition_pattern(const struct key_value_info *kvi,
|
|||||||
if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {
|
if (pat->buf[0] == '.' && is_dir_sep(pat->buf[1])) {
|
||||||
const char *slash;
|
const char *slash;
|
||||||
|
|
||||||
if (!kvi || !kvi->path)
|
if (!kvi || kvi->origin_type != CONFIG_ORIGIN_FILE)
|
||||||
return error(_("relative config include "
|
return error(_("relative config include "
|
||||||
"conditionals must come from files"));
|
"conditionals must come from files"));
|
||||||
|
|
||||||
strbuf_realpath(&path, kvi->path, 1);
|
strbuf_realpath(&path, kvi->filename, 1);
|
||||||
slash = find_last_dir_sep(path.buf);
|
slash = find_last_dir_sep(path.buf);
|
||||||
if (!slash)
|
if (!slash)
|
||||||
BUG("how is this possible?");
|
BUG("how is this possible?");
|
||||||
@@ -633,7 +632,6 @@ void kvi_from_param(struct key_value_info *out)
|
|||||||
out->linenr = -1;
|
out->linenr = -1;
|
||||||
out->origin_type = CONFIG_ORIGIN_CMDLINE;
|
out->origin_type = CONFIG_ORIGIN_CMDLINE;
|
||||||
out->scope = CONFIG_SCOPE_COMMAND;
|
out->scope = CONFIG_SCOPE_COMMAND;
|
||||||
out->path = NULL;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
int git_config_parse_parameter(const char *text,
|
int git_config_parse_parameter(const char *text,
|
||||||
@@ -1036,7 +1034,6 @@ static void kvi_from_source(struct config_source *cs,
|
|||||||
out->origin_type = cs->origin_type;
|
out->origin_type = cs->origin_type;
|
||||||
out->linenr = cs->linenr;
|
out->linenr = cs->linenr;
|
||||||
out->scope = scope;
|
out->scope = scope;
|
||||||
out->path = cs->path;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
static int git_parse_source(struct config_source *cs, config_fn_t fn,
|
static int git_parse_source(struct config_source *cs, config_fn_t fn,
|
||||||
@@ -1855,17 +1852,19 @@ static int do_config_from(struct config_source *top, config_fn_t fn,
|
|||||||
|
|
||||||
static int do_config_from_file(config_fn_t fn,
|
static int do_config_from_file(config_fn_t fn,
|
||||||
const enum config_origin_type origin_type,
|
const enum config_origin_type origin_type,
|
||||||
const char *name, const char *path, FILE *f,
|
const char *name, FILE *f, void *data,
|
||||||
void *data, enum config_scope scope,
|
enum config_scope scope,
|
||||||
const struct config_options *opts)
|
const struct config_options *opts)
|
||||||
{
|
{
|
||||||
struct config_source top = CONFIG_SOURCE_INIT;
|
struct config_source top = CONFIG_SOURCE_INIT;
|
||||||
int ret;
|
int ret;
|
||||||
|
|
||||||
|
if (origin_type == CONFIG_ORIGIN_FILE && (!name || !*name))
|
||||||
|
BUG("missing filename for CONFIG_ORIGIN_FILE");
|
||||||
|
|
||||||
top.u.file = f;
|
top.u.file = f;
|
||||||
top.origin_type = origin_type;
|
top.origin_type = origin_type;
|
||||||
top.name = name;
|
top.name = name;
|
||||||
top.path = path;
|
|
||||||
top.default_error_action = CONFIG_ERROR_DIE;
|
top.default_error_action = CONFIG_ERROR_DIE;
|
||||||
top.do_fgetc = config_file_fgetc;
|
top.do_fgetc = config_file_fgetc;
|
||||||
top.do_ungetc = config_file_ungetc;
|
top.do_ungetc = config_file_ungetc;
|
||||||
@@ -1880,8 +1879,8 @@ static int do_config_from_file(config_fn_t fn,
|
|||||||
static int git_config_from_stdin(config_fn_t fn, void *data,
|
static int git_config_from_stdin(config_fn_t fn, void *data,
|
||||||
enum config_scope scope)
|
enum config_scope scope)
|
||||||
{
|
{
|
||||||
return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", NULL, stdin,
|
return do_config_from_file(fn, CONFIG_ORIGIN_STDIN, "", stdin, data,
|
||||||
data, scope, NULL);
|
scope, NULL);
|
||||||
}
|
}
|
||||||
|
|
||||||
int git_config_from_file_with_options(config_fn_t fn, const char *filename,
|
int git_config_from_file_with_options(config_fn_t fn, const char *filename,
|
||||||
@@ -1896,7 +1895,7 @@ int git_config_from_file_with_options(config_fn_t fn, const char *filename,
|
|||||||
f = fopen_or_warn(filename, "r");
|
f = fopen_or_warn(filename, "r");
|
||||||
if (f) {
|
if (f) {
|
||||||
ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename,
|
ret = do_config_from_file(fn, CONFIG_ORIGIN_FILE, filename,
|
||||||
filename, f, data, scope, opts);
|
f, data, scope, opts);
|
||||||
fclose(f);
|
fclose(f);
|
||||||
}
|
}
|
||||||
return ret;
|
return ret;
|
||||||
@@ -1921,7 +1920,6 @@ int git_config_from_mem(config_fn_t fn,
|
|||||||
top.u.buf.pos = 0;
|
top.u.buf.pos = 0;
|
||||||
top.origin_type = origin_type;
|
top.origin_type = origin_type;
|
||||||
top.name = name;
|
top.name = name;
|
||||||
top.path = NULL;
|
|
||||||
top.default_error_action = CONFIG_ERROR_ERROR;
|
top.default_error_action = CONFIG_ERROR_ERROR;
|
||||||
top.do_fgetc = config_buf_fgetc;
|
top.do_fgetc = config_buf_fgetc;
|
||||||
top.do_ungetc = config_buf_ungetc;
|
top.do_ungetc = config_buf_ungetc;
|
||||||
|
|||||||
2
config.h
2
config.h
@@ -122,14 +122,12 @@ struct key_value_info {
|
|||||||
int linenr;
|
int linenr;
|
||||||
enum config_origin_type origin_type;
|
enum config_origin_type origin_type;
|
||||||
enum config_scope scope;
|
enum config_scope scope;
|
||||||
const char *path;
|
|
||||||
};
|
};
|
||||||
#define KVI_INIT { \
|
#define KVI_INIT { \
|
||||||
.filename = NULL, \
|
.filename = NULL, \
|
||||||
.linenr = -1, \
|
.linenr = -1, \
|
||||||
.origin_type = CONFIG_ORIGIN_UNKNOWN, \
|
.origin_type = CONFIG_ORIGIN_UNKNOWN, \
|
||||||
.scope = CONFIG_SCOPE_UNKNOWN, \
|
.scope = CONFIG_SCOPE_UNKNOWN, \
|
||||||
.path = NULL, \
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/* Captures additional information that a config callback can use. */
|
/* Captures additional information that a config callback can use. */
|
||||||
|
|||||||
Reference in New Issue
Block a user