Skip to content

Commit

Permalink
Merge 'gvfs/jk/submodule-fix-loose'
Browse files Browse the repository at this point in the history
This backports the security bug fix for recursive submodule cloning.
  • Loading branch information
dscho committed Jun 7, 2018
2 parents 93d3c89 + b983794 commit 093813a
Show file tree
Hide file tree
Showing 17 changed files with 473 additions and 42 deletions.
4 changes: 2 additions & 2 deletions apply.c
Original file line number Diff line number Diff line change
Expand Up @@ -3878,9 +3878,9 @@ static int check_unsafe_path(struct patch *patch)
if (!patch->is_delete)
new_name = patch->new_name;

if (old_name && !verify_path(old_name))
if (old_name && !verify_path(old_name, patch->old_mode))
return error(_("invalid path '%s'"), old_name);
if (new_name && !verify_path(new_name))
if (new_name && !verify_path(new_name, patch->new_mode))
return error(_("invalid path '%s'"), new_name);
return 0;
}
Expand Down
24 changes: 24 additions & 0 deletions builtin/submodule--helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -1817,6 +1817,29 @@ static int is_active(int argc, const char **argv, const char *prefix)
return !is_submodule_active(the_repository, argv[1]);
}

/*
* Exit non-zero if any of the submodule names given on the command line is
* invalid. If no names are given, filter stdin to print only valid names
* (which is primarily intended for testing).
*/
static int check_name(int argc, const char **argv, const char *prefix)
{
if (argc > 1) {
while (*++argv) {
if (check_submodule_name(*argv) < 0)
return 1;
}
} else {
struct strbuf buf = STRBUF_INIT;
while (strbuf_getline(&buf, stdin) != EOF) {
if (!check_submodule_name(buf.buf))
printf("%s\n", buf.buf);
}
strbuf_release(&buf);
}
return 0;
}

#define SUPPORT_SUPER_PREFIX (1<<0)

struct cmd_struct {
Expand All @@ -1842,6 +1865,7 @@ static struct cmd_struct commands[] = {
{"push-check", push_check, 0},
{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
{"is-active", is_active, 0},
{"check-name", check_name, 0},
};

int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
Expand Down
31 changes: 20 additions & 11 deletions builtin/update-index.c
Original file line number Diff line number Diff line change
Expand Up @@ -363,10 +363,9 @@ static int process_directory(const char *path, int len, struct stat *st)
return error("%s: is a directory - add files inside instead", path);
}

static int process_path(const char *path)
static int process_path(const char *path, struct stat *st, int stat_errno)
{
int pos, len;
struct stat st;
const struct cache_entry *ce;

len = strlen(path);
Expand All @@ -390,13 +389,13 @@ static int process_path(const char *path)
* First things first: get the stat information, to decide
* what to do about the pathname!
*/
if (lstat(path, &st) < 0)
return process_lstat_error(path, errno);
if (stat_errno)
return process_lstat_error(path, stat_errno);

if (S_ISDIR(st.st_mode))
return process_directory(path, len, &st);
if (S_ISDIR(st->st_mode))
return process_directory(path, len, st);

return add_one_path(ce, path, len, &st);
return add_one_path(ce, path, len, st);
}

static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
Expand All @@ -405,7 +404,7 @@ static int add_cacheinfo(unsigned int mode, const struct object_id *oid,
int len, option;
struct cache_entry *ce;

if (!verify_path(path))
if (!verify_path(path, mode))
return error("Invalid path '%s'", path);

len = strlen(path);
Expand Down Expand Up @@ -447,7 +446,17 @@ static void chmod_path(char flip, const char *path)

static void update_one(const char *path)
{
if (!verify_path(path)) {
int stat_errno = 0;
struct stat st;

if (mark_valid_only || mark_skip_worktree_only || force_remove)
st.st_mode = 0;
else if (lstat(path, &st) < 0) {
st.st_mode = 0;
stat_errno = errno;
} /* else stat is valid */

if (!verify_path(path, st.st_mode)) {
fprintf(stderr, "Ignoring path %s\n", path);
return;
}
Expand All @@ -473,7 +482,7 @@ static void update_one(const char *path)
report("remove '%s'", path);
return;
}
if (process_path(path))
if (process_path(path, &st, stat_errno))
die("Unable to process path %s", path);
report("add '%s'", path);
}
Expand Down Expand Up @@ -543,7 +552,7 @@ static void read_index_info(int nul_term_line)
path_name = uq.buf;
}

if (!verify_path(path_name)) {
if (!verify_path(path_name, mode)) {
fprintf(stderr, "Ignoring path %s\n", path_name);
continue;
}
Expand Down
12 changes: 10 additions & 2 deletions cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ extern int unmerged_index(const struct index_state *);
*/
extern int index_has_changes(struct strbuf *sb);

extern int verify_path(const char *path);
extern int verify_path(const char *path, unsigned mode);
extern int strcmp_offset(const char *s1, const char *s2, size_t *first_change);
extern int index_dir_exists(struct index_state *istate, const char *name, int namelen);
extern void adjust_dirname_case(struct index_state *istate, char *name);
Expand Down Expand Up @@ -1185,7 +1185,15 @@ int normalize_path_copy(char *dst, const char *src);
int longest_ancestor_length(const char *path, struct string_list *prefixes);
char *strip_path_suffix(const char *path, const char *suffix);
int daemon_avoid_alias(const char *path);
extern int is_ntfs_dotgit(const char *name);

/*
* These functions match their is_hfs_dotgit() counterparts; see utf8.h for
* details.
*/
int is_ntfs_dotgit(const char *name);
int is_ntfs_dotgitmodules(const char *name);
int is_ntfs_dotgitignore(const char *name);
int is_ntfs_dotgitattributes(const char *name);

/*
* Returns true iff "str" could be confused as a command-line option when
Expand Down
2 changes: 1 addition & 1 deletion dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -3065,7 +3065,7 @@ void untracked_cache_invalidate_path(struct index_state *istate,
{
if (!istate->untracked || !istate->untracked->root)
return;
if (!safe_path && !verify_path(path))
if (!safe_path && !verify_path(path, 0))
return;
invalidate_one_component(istate->untracked, istate->untracked->root,
path, strlen(path));
Expand Down
17 changes: 17 additions & 0 deletions git-compat-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,23 @@ static inline int sane_iscase(int x, int is_lower)
return (x & 0x20) == 0;
}

/*
* Like skip_prefix, but compare case-insensitively. Note that the comparison
* is done via tolower(), so it is strictly ASCII (no multi-byte characters or
* locale-specific conversions).
*/
static inline int skip_iprefix(const char *str, const char *prefix,
const char **out)
{
do {
if (!*prefix) {
*out = str;
return 1;
}
} while (tolower(*str++) == tolower(*prefix++));
return 0;
}

static inline int strtoul_ui(char const *s, int base, unsigned int *result)
{
unsigned long ul;
Expand Down
5 changes: 5 additions & 0 deletions git-submodule.sh
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,11 @@ Use -f if you really want to add it." >&2
sm_name="$sm_path"
fi

if ! git submodule--helper check-name "$sm_name"
then
die "$(eval_gettext "'$sm_name' is not a valid submodule name")"
fi

# perhaps the path exists and is already a git repo, else clone it
if test -e "$sm_path"
then
Expand Down
86 changes: 85 additions & 1 deletion path.c
Original file line number Diff line number Diff line change
Expand Up @@ -1310,7 +1310,7 @@ static int only_spaces_and_periods(const char *path, size_t len, size_t skip)

int is_ntfs_dotgit(const char *name)
{
int len;
size_t len;

for (len = 0; ; len++)
if (!name[len] || name[len] == '\\' || is_dir_sep(name[len])) {
Expand All @@ -1327,6 +1327,90 @@ int is_ntfs_dotgit(const char *name)
}
}

static int is_ntfs_dot_generic(const char *name,
const char *dotgit_name,
size_t len,
const char *dotgit_ntfs_shortname_prefix)
{
int saw_tilde;
size_t i;

if ((name[0] == '.' && !strncasecmp(name + 1, dotgit_name, len))) {
i = len + 1;
only_spaces_and_periods:
for (;;) {
char c = name[i++];
if (!c)
return 1;
if (c != ' ' && c != '.')
return 0;
}
}

/*
* Is it a regular NTFS short name, i.e. shortened to 6 characters,
* followed by ~1, ... ~4?
*/
if (!strncasecmp(name, dotgit_name, 6) && name[6] == '~' &&
name[7] >= '1' && name[7] <= '4') {
i = 8;
goto only_spaces_and_periods;
}

/*
* Is it a fall-back NTFS short name (for details, see
* https://en.wikipedia.org/wiki/8.3_filename?
*/
for (i = 0, saw_tilde = 0; i < 8; i++)
if (name[i] == '\0')
return 0;
else if (saw_tilde) {
if (name[i] < '0' || name[i] > '9')
return 0;
} else if (name[i] == '~') {
if (name[++i] < '1' || name[i] > '9')
return 0;
saw_tilde = 1;
} else if (i >= 6)
return 0;
else if (name[i] < 0) {
/*
* We know our needles contain only ASCII, so we clamp
* here to make the results of tolower() sane.
*/
return 0;
} else if (tolower(name[i]) != dotgit_ntfs_shortname_prefix[i])
return 0;

goto only_spaces_and_periods;
}

/*
* Inline helper to make sure compiler resolves strlen() on literals at
* compile time.
*/
static inline int is_ntfs_dot_str(const char *name, const char *dotgit_name,
const char *dotgit_ntfs_shortname_prefix)
{
return is_ntfs_dot_generic(name, dotgit_name, strlen(dotgit_name),
dotgit_ntfs_shortname_prefix);
}

int is_ntfs_dotgitmodules(const char *name)
{
return is_ntfs_dot_str(name, "gitmodules", "gi7eba");
}

int is_ntfs_dotgitignore(const char *name)
{
return is_ntfs_dot_str(name, "gitignore", "gi250a");
}

int is_ntfs_dotgitattributes(const char *name)
{
return is_ntfs_dot_str(name, "gitattributes", "gi7d29");
}

int looks_like_command_line_option(const char *str)
{
return str && str[0] == '-';
Expand Down
51 changes: 38 additions & 13 deletions read-cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ struct cache_entry *make_cache_entry(struct index_state *istate,
int len;
struct cache_entry *ce, *ret;

if (!verify_path(path)) {
if (!verify_path(path, mode)) {
error("Invalid path '%s'", path);
return NULL;
}
Expand Down Expand Up @@ -885,7 +885,7 @@ int ce_same_name(const struct cache_entry *a, const struct cache_entry *b)
* Also, we don't want double slashes or slashes at the
* end that can make pathnames ambiguous.
*/
static int verify_dotfile(const char *rest)
static int verify_dotfile(const char *rest, unsigned mode)
{
/*
* The first character was '.', but that
Expand All @@ -899,25 +899,37 @@ static int verify_dotfile(const char *rest)

switch (*rest) {
/*
* ".git" followed by NUL or slash is bad. This
* shares the path end test with the ".." case.
* ".git" followed by NUL or slash is bad. Note that we match
* case-insensitively here, even if ignore_case is not set.
* This outlaws ".GIT" everywhere out of an abundance of caution,
* since there's really no good reason to allow it.
*
* Once we've seen ".git", we can also find ".gitmodules", etc (also
* case-insensitively).
*/
case 'g':
case 'G':
if (rest[1] != 'i' && rest[1] != 'I')
break;
if (rest[2] != 't' && rest[2] != 'T')
break;
rest += 2;
/* fallthrough */
if (rest[3] == '\0' || is_dir_sep(rest[3]))
return 0;
if (S_ISLNK(mode)) {
rest += 3;
if (skip_iprefix(rest, "modules", &rest) &&
(*rest == '\0' || is_dir_sep(*rest)))
return 0;
}
break;
case '.':
if (rest[1] == '\0' || is_dir_sep(rest[1]))
return 0;
}
return 1;
}

int verify_path(const char *path)
int verify_path(const char *path, unsigned mode)
{
char c;

Expand All @@ -930,12 +942,25 @@ int verify_path(const char *path)
return 1;
if (is_dir_sep(c)) {
inside:
if (protect_hfs && is_hfs_dotgit(path))
return 0;
if (protect_ntfs && is_ntfs_dotgit(path))
return 0;
if (protect_hfs) {
if (is_hfs_dotgit(path))
return 0;
if (S_ISLNK(mode)) {
if (is_hfs_dotgitmodules(path))
return 0;
}
}
if (protect_ntfs) {
if (is_ntfs_dotgit(path))
return 0;
if (S_ISLNK(mode)) {
if (is_ntfs_dotgitmodules(path))
return 0;
}
}

c = *path++;
if ((c == '.' && !verify_dotfile(path)) ||
if ((c == '.' && !verify_dotfile(path, mode)) ||
is_dir_sep(c) || c == '\0')
return 0;
}
Expand Down Expand Up @@ -1252,7 +1277,7 @@ static int add_index_entry_with_check(struct index_state *istate, struct cache_e

if (!ok_to_add)
return -1;
if (!verify_path(ce->name))
if (!verify_path(ce->name, ce->ce_mode))
return error("Invalid path '%s'", ce->name);

if (!skip_df_check &&
Expand Down
Loading

0 comments on commit 093813a

Please sign in to comment.