login
Header Space

 
 

[RFH/PATCH] prefix_path(): disallow absolute paths

Score:
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Johannes Sixt <j.sixt@...>
Cc: Junio C Hamano <gitster@...>, Shawn Bohrer <shawn.bohrer@...>, <git@...>
Date: Monday, January 28, 2008 - 8:33 am

Without this fix, "git ls-files --others /" would list _all_ files,
except for those tracked in the current repository.  Worse, "git clean /"
would start removing them.

Noticed by Johannes Sixt.

Incidentally, it fixes some strange code in builtin-mv.c by yours truly,
where a slash was added to "dst" but then ignored, and instead taken from
the source path.  This triggered the new check for absolute paths.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	On Mon, 28 Jan 2008, Johannes Sixt wrote:

	> Junio C Hamano schrieb:
	> > Johannes Sixt <j.sixt@viscovery.net> writes:
	> > 
	> >> The "problem" is not only with git-clean, but also in others, 
	> >> like git-ls-files. Try this in you favorite repository:
	> >>
	> >>    $ git ls-files -o /*bin
	> >>
	> >> The output does not make a lot of sense. (Here it lists the 
	> >> contents of /bin and /sbin.) Not that it hurts with ls-files, 
	> >> but
	> >>
	> >>    $ git clean -f /
	> >>
	> >> is basically a synonym for
	> >>
	> >>    $ rm -rf /
	> > 
	> > Yeah, /*bin is not inside the repository so it should not even 
	> > be reported as "others".  Shouldn't the commands detect this 
	> > and reject feeding such paths outside the work tree to the 
	> > core, which always expect you to talk about paths inside?
	> 
	> That's what I had expected. But look:
	> 
	>    $ git ls-files -o /
	>    [... tons of file names ...]
	> 
	>    $ git ls-files -o ..
	>    fatal: '..' is outside repository
	> 
	>    $ git clean -n /    # with Shawn's patch
	>    Would remove /bin/
	>    [... etc ...]
	> 
	>    $ git clean -n ..
	>    fatal: '..' is outside repository
	> 
	> Some mechanism for this is already there; it's just not complete 
	> enough.

	This patch cannot be applied as-is: t3101 is failing (t7001 is 
	fixed by the builtin-mv.c part).

	The failure of t3101 has something to do with ls-tree filtering 
	out invalid paths; I maintain that this behaviour is wrong to 
	begin with.

	So the help I am requesting is this: so late in the game for 1.5.4 
	I would hate to introduce a change in prefix_path(), because it 
	affects apparently too much.  However, the "git clean /" bug is a 
	real one, and should at least be prevented.  What to do?

 builtin-mv.c |    4 ++--
 setup.c      |    2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin-mv.c b/builtin-mv.c
index 990e213..94f6dd2 100644
--- a/builtin-mv.c
+++ b/builtin-mv.c
@@ -164,7 +164,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 				}
 
 				dst = add_slash(dst);
-				dst_len = strlen(dst) - 1;
+				dst_len = strlen(dst);
 
 				for (j = 0; j < last - first; j++) {
 					const char *path =
@@ -172,7 +172,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
 					source[argc + j] = path;
 					destination[argc + j] =
 						prefix_path(dst, dst_len,
-							path + length);
+							path + length + 1);
 					modes[argc + j] = INDEX;
 				}
 				argc += last - first;
diff --git a/setup.c b/setup.c
index 2174e78..5a4aadc 100644
--- a/setup.c
+++ b/setup.c
@@ -13,6 +13,8 @@ const char *get_current_prefix()
 const char *prefix_path(const char *prefix, int len, const char *path)
 {
 	const char *orig = path;
+	if (is_absolute_path(path))
+		die("no absolute paths allowed: '%s'", path);
 	for (;;) {
 		char c;
 		if (*path != '.')
-- 
1.5.4.rc5.15.g8231f

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
git-clean buglet, Johannes Sixt, (Wed Jan 23, 11:14 am)
Re: git-clean buglet, Johannes Schindelin, (Wed Jan 23, 11:29 am)
Re: git-clean buglet, Johannes Sixt, (Wed Jan 23, 11:40 am)
[PATCH] Fix off by one error in prep_exclude., Shawn Bohrer, (Sun Jan 27, 3:55 pm)
Re: [PATCH] Fix off by one error in prep_exclude., Johannes Schindelin, (Sun Jan 27, 4:44 pm)
Re: [PATCH] Fix off by one error in prep_exclude., Junio C Hamano, (Sun Jan 27, 6:34 pm)
Re: [PATCH] Fix off by one error in prep_exclude., Shawn Bohrer, (Sun Jan 27, 8:34 pm)
Re: [PATCH] Fix off by one error in prep_exclude., Junio C Hamano, (Sun Jan 27, 10:52 pm)
Re: [PATCH] Fix off by one error in prep_exclude., Johannes Sixt, (Mon Jan 28, 3:12 am)
Re: [PATCH] Fix off by one error in prep_exclude., Junio C Hamano, (Mon Jan 28, 4:46 am)
Re: [PATCH] Fix off by one error in prep_exclude., Johannes Sixt, (Mon Jan 28, 5:05 am)
[RFH/PATCH] prefix_path(): disallow absolute paths, Johannes Schindelin, (Mon Jan 28, 8:33 am)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Junio C Hamano, (Mon Jan 28, 9:23 pm)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, しらいしななこ, (Tue Jan 29, 5:53 pm)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Junio C Hamano, (Tue Jan 29, 8:43 pm)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Johannes Sixt, (Tue Jan 29, 3:20 am)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Junio C Hamano, (Tue Jan 29, 3:28 am)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Johannes Sixt, (Tue Jan 29, 3:43 am)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Junio C Hamano, (Tue Jan 29, 4:31 am)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Johannes Schindelin, (Mon Jan 28, 10:37 pm)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Junio C Hamano, (Mon Jan 28, 10:45 pm)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Johannes Schindelin, (Mon Jan 28, 10:59 pm)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Junio C Hamano, (Mon Jan 28, 10:03 pm)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Junio C Hamano, (Tue Jan 29, 3:02 am)
[PATCH] More test cases for sanitized path names, Robin Rosenberg, (Fri Feb 1, 12:34 am)
Re: [PATCH] More test cases for sanitized path names, Junio C Hamano, (Fri Feb 1, 3:17 am)
[PATCH for post 1.5.4] Sane use of test_expect_failure, Junio C Hamano, (Fri Feb 1, 5:50 am)
Re: [PATCH] Sane use of test_expect_failure, Junio C Hamano, (Sat Feb 2, 6:06 am)
Re: [PATCH] More test cases for sanitized path names, Robin Rosenberg, (Fri Feb 1, 5:10 am)
Re: [PATCH] More test cases for sanitized path names, Junio C Hamano, (Fri Feb 1, 6:22 am)
Re: [PATCH] More test cases for sanitized path names, Robin Rosenberg, (Fri Feb 1, 10:17 am)
Re: [PATCH] More test cases for sanitized path names, Junio C Hamano, (Fri Feb 1, 1:45 pm)
Re: [PATCH] More test cases for sanitized path names, Junio C Hamano, (Fri Feb 1, 6:51 am)
Re: [PATCH] More test cases for sanitized path names, Junio C Hamano, (Fri Feb 1, 7:10 am)
[PATCH] Make blame accept absolute paths, Robin Rosenberg, (Fri Feb 1, 12:07 am)
Re: [RFH/PATCH] prefix_path(): disallow absolute paths, Junio C Hamano, (Mon Jan 28, 10:03 pm)
[PATCH] prefix_path(): disallow absolute paths, Johannes Schindelin, (Mon Jan 28, 11:05 am)
Re: [PATCH] Fix off by one error in prep_exclude., Junio C Hamano, (Mon Jan 28, 5:22 am)
[PATCH] Fix off by one error in prep_exclude., Shawn Bohrer, (Sun Jan 27, 8:37 pm)
Re: [PATCH] Fix off by one error in prep_exclude., Johannes Schindelin, (Mon Jan 28, 7:59 am)
Re: [PATCH] Fix off by one error in prep_exclude., Junio C Hamano, (Mon Jan 28, 8:04 am)
Re: [PATCH] Fix off by one error in prep_exclude., Shawn Bohrer, (Sun Jan 27, 5:15 pm)
Re: git-clean buglet, Johannes Sixt, (Wed Jan 23, 11:24 am)
speck-geostationary