[PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR

Subsystems: the rest

DORMANTno replies

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:52:14

We have always dwimmed the user input $string into a ref by first looking
directly inside $GIT_DIR, and then in $GIT_DIR/refs, $GIT_DIR/refs/tags,
etc., and that is what made

    git log HEAD..MERGE_HEAD

work correctly. This however means that

    git rev-parse config
    git log index

would look at $GIT_DIR/config and $GIT_DIR/index and see if they are valid
refs.

To reduce confusion, let's not dwim a path immediately below $GIT_DIR that
is not all-caps.

Signed-off-by: Junio C Hamano <redacted>
---

 * And this adds coverage to refname_match() and shorten_unambiguous_ref()
   on top of the one from yesterday.
 
 refs.c |   36 ++++++++++++++++++++++++++++++++++--
 1 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index e3692bd..e54c482 100644
--- a/refs.c
+++ b/refs.c

@@ -994,12 +994,34 @@ const char *ref_fetch_rules[] = {
 	NULL
 };
 
+static int refname_ok_at_root_level(const char *str, int len)
+{
+	int seen_non_root_char = 0;
+
+	while (len--) {
+		char ch = *str++;
+
+		if (ch == '/')
+			return 1;
+		/*
+		 * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
+		 * the root level as a ref.
+		 */
+		if (ch != '_' && (ch < 'A' || 'Z' < ch))
+			seen_non_root_char = 1;
+	}
+	return !seen_non_root_char;
+}
+
 int refname_match(const char *abbrev_name, const char *full_name, const char **rules)
 {
 	const char **p;
 	const int abbrev_name_len = strlen(abbrev_name);
 
 	for (p = rules; *p; p++) {
+		if (p == rules &&
+		    !refname_ok_at_root_level(abbrev_name, abbrev_name_len))
+			continue;
 		if (!strcmp(full_name, mkpath(*p, abbrev_name_len, abbrev_name))) {
 			return 1;
 		}

@@ -1100,6 +1122,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 		unsigned char *this_result;
 		int flag;
 
+		if (p == ref_rev_parse_rules && !refname_ok_at_root_level(str, len))
+			continue;
 		this_result = refs_found ? sha1_from_ref : sha1;
 		mksnpath(fullref, sizeof(fullref), *p, len, str);
 		r = resolve_ref(fullref, this_result, 1, &flag);

@@ -1128,6 +1152,8 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 		char path[PATH_MAX];
 		const char *ref, *it;
 
+		if (p == ref_rev_parse_rules && !refname_ok_at_root_level(str, len))
+			continue;
 		mksnpath(path, sizeof(path), *p, len, str);
 		ref = resolve_ref(path, hash, 1, NULL);
 		if (!ref)

@@ -2045,12 +2071,14 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
 	/* buffer for scanf result, at most ref must fit */
 	short_name = xstrdup(ref);
 
-	/* skip first rule, it will always match */
-	for (i = nr_rules - 1; i > 0 ; --i) {
+	for (i = nr_rules - 1; i >= 0; i--) {
 		int j;
 		int rules_to_fail = i;
 		int short_name_len;
 
+		if (!i && !refname_ok_at_root_level(ref, strlen(ref)))
+			continue;
+
 		if (1 != sscanf(ref, scanf_fmts[i], short_name))
 			continue;

@@ -2076,6 +2104,10 @@ char *shorten_unambiguous_ref(const char *ref, int strict)
 			if (i == j)
 				continue;
 
+			if (!j &&
+			    !refname_ok_at_root_level(short_name, short_name_len))
+				continue;
+
 			/*
 			 * the short name is ambiguous, if it resolves
 			 * (with this previous rule) to a valid ref

-- 
1.7.7.213.g8b0e1

Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR

From: Michael Haggerty <hidden>
Date: 2016-06-15 22:52:14

On 10/12/2011 07:49 PM, Junio C Hamano wrote:

quoted hunk

diff --git a/refs.c b/refs.c
index e3692bd..e54c482 100644
--- a/refs.c
+++ b/refs.c

@@ -994,12 +994,34 @@ const char *ref_fetch_rules[] = {
 	NULL
 };
 
+static int refname_ok_at_root_level(const char *str, int len)
+{
+	int seen_non_root_char = 0;
+
+	while (len--) {
+		char ch = *str++;
+
+		if (ch == '/')
+			return 1;
+		/*
+		 * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
+		 * the root level as a ref.
+		 */
+		if (ch != '_' && (ch < 'A' || 'Z' < ch))
+			seen_non_root_char = 1;
+	}
+	return !seen_non_root_char;
+}
+

Nit: the seen_non_root_char variable can be replaced by an early "return
0" from the loop and "return 1" if the loop falls through.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH 2/2] Restrict ref-like names immediately below $GIT_DIR

From: Jeff King <hidden>
Date: 2016-06-15 22:52:14

On Wed, Oct 12, 2011 at 10:49:41AM -0700, Junio C Hamano wrote:

+static int refname_ok_at_root_level(const char *str, int len)
+{
+	int seen_non_root_char = 0;
+
+	while (len--) {
+		char ch = *str++;
+
+		if (ch == '/')
+			return 1;
+		/*
+		 * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
+		 * the root level as a ref.
+		 */
+		if (ch != '_' && (ch < 'A' || 'Z' < ch))
+			seen_non_root_char = 1;
+	}
+	return !seen_non_root_char;
+}

I thought from your earlier comment:

I wanted to start as loose as possible to avoid negatively impacting
existing users, later to tighten.  As fsck and friends never look
outside of refs/, I think the prefix refs/ is a reasonable restriction
that is safe.

that you did agree with tightening this up to allow just refs/ as a
subdirectory.

Squashable patch is below.

diff --git a/refs.c b/refs.c
index 0f26d9d..b159c4a 100644
--- a/refs.c
+++ b/refs.c

@@ -994,21 +994,20 @@ int check_refname_format(const char *ref, int flags)
 
 static int refname_ok_at_root_level(const char *str, int len)
 {
-	int seen_non_root_char = 0;
+	if (len >= 5 && !memcmp(str, "refs/", 5))
+		return 1;
 
 	while (len--) {
 		char ch = *str++;
 
-		if (ch == '/')
-			return 1;
 		/*
 		 * Only accept likes of .git/HEAD, .git/MERGE_HEAD at
 		 * the root level as a ref.
 		 */
 		if (ch != '_' && (ch < 'A' || 'Z' < ch))
-			seen_non_root_char = 1;
+			return 0;
 	}
-	return !seen_non_root_char;
+	return 1;
 }
 
 int refname_match(const char *abbrev_name, const char *full_name, const char **rules)

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help