Commit b93e82d4 by Carlos Martín Nieto Committed by Patrick Steinhardt

submodule: ignore path and url attributes if they look like options

These can be used to inject options in an implementation which performs a
recursive clone by executing an external command via crafted url and path
attributes such that it triggers a local executable to be run.

The library is not vulnerable as we do not rely on external executables but a
user of the library might be relying on that so we add this protection.

This matches this aspect of git's fix for CVE-2018-17456.
parent 7e8d9789
...@@ -1792,6 +1792,14 @@ static int get_value(const char **out, git_config *cfg, git_buf *buf, const char ...@@ -1792,6 +1792,14 @@ static int get_value(const char **out, git_config *cfg, git_buf *buf, const char
return error; return error;
} }
static bool looks_like_command_line_option(const char *s)
{
if (s && s[0] == '-')
return true;
return false;
}
static int submodule_read_config(git_submodule *sm, git_config *cfg) static int submodule_read_config(git_submodule *sm, git_config *cfg)
{ {
git_buf key = GIT_BUF_INIT; git_buf key = GIT_BUF_INIT;
...@@ -1805,24 +1813,31 @@ static int submodule_read_config(git_submodule *sm, git_config *cfg) ...@@ -1805,24 +1813,31 @@ static int submodule_read_config(git_submodule *sm, git_config *cfg)
if ((error = get_value(&value, cfg, &key, sm->name, "path")) == 0) { if ((error = get_value(&value, cfg, &key, sm->name, "path")) == 0) {
in_config = 1; in_config = 1;
/* We would warn here if we had that API */
if (!looks_like_command_line_option(value)) {
/* /*
* TODO: if case insensitive filesystem, then the following strcmp * TODO: if case insensitive filesystem, then the following strcmp
* should be strcasecmp * should be strcasecmp
*/ */
if (strcmp(sm->name, value) != 0) { if (strcmp(sm->name, value) != 0) {
if (sm->path != sm->name) if (sm->path != sm->name)
git__free(sm->path); git__free(sm->path);
sm->path = git__strdup(value); sm->path = git__strdup(value);
GITERR_CHECK_ALLOC(sm->path); GITERR_CHECK_ALLOC(sm->path);
}
} }
} else if (error != GIT_ENOTFOUND) { } else if (error != GIT_ENOTFOUND) {
goto cleanup; goto cleanup;
} }
if ((error = get_value(&value, cfg, &key, sm->name, "url")) == 0) { if ((error = get_value(&value, cfg, &key, sm->name, "url")) == 0) {
in_config = 1; /* We would warn here if we had that API */
sm->url = git__strdup(value); if (!looks_like_command_line_option(value)) {
GITERR_CHECK_ALLOC(sm->url); in_config = 1;
sm->url = git__strdup(value);
GITERR_CHECK_ALLOC(sm->url);
}
} else if (error != GIT_ENOTFOUND) { } else if (error != GIT_ENOTFOUND) {
goto cleanup; goto cleanup;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment