Re: [PATCH 1/5] Library code for user-relative paths, take three.

exon@op5.se (Andreas Ericsson) writes:

+	if(strict && *dir != '/')

(style everywhere)

	if (strict ...

+	/* This is perfectly safe, and people tend to think of the directory
+	 * where they ran git-init-db as their repository, so humour them. */
+	(void)chdir(".git");

It might be safe, but I think it changes the behaviour of
upload-pack with strict case.  My gut reaction is we would want
"if (!strict)" in front.  Thoughts?

Junio C Hamano wrote:

quoted

+	/* This is perfectly safe, and people tend to think of the directory
+	 * where they ran git-init-db as their repository, so humour them. */
+	(void)chdir(".git");

It might be safe, but I think it changes the behaviour of
upload-pack with strict case.  My gut reaction is we would want
"if (!strict)" in front.  Thoughts?

As it says in the comment; People tend to think of the directory where 
they ran "git init-db" as their repository, so humour them. It's nice 
for sharing files between devs in the office, and it *is* safe. Do as 
you please though. It's the generality of the

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Andreas Ericsson wrote:

Junio C Hamano wrote:

quoted

quoted

+    /* This is perfectly safe, and people tend to think of the 
directory
+     * where they ran git-init-db as their repository, so humour 
them. */
+    (void)chdir(".git");

It might be safe, but I think it changes the behaviour of
upload-pack with strict case.  My gut reaction is we would want
"if (!strict)" in front.  Thoughts?

As it says in the comment; People tend to think of the directory where 
they ran "git init-db" as their repository, so humour them. It's nice 
for sharing files between devs in the office, and it *is* safe. Do as 
you please though. It's the generality of the

Butter-fingers be me. Sorry about that.

What I meant to say was that:

"it's the general idea of the patchset I'm after".

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Andreas Ericsson wrote:

Junio C Hamano wrote:

quoted

quoted

+    /* This is perfectly safe, and people tend to think of the 
directory
+     * where they ran git-init-db as their repository, so humour 
them. */
+    (void)chdir(".git");

It might be safe, but I think it changes the behaviour of
upload-pack with strict case.  My gut reaction is we would want
"if (!strict)" in front.  Thoughts?

As it says in the comment; People tend to think of the directory where 
they ran "git init-db" as their repository, so humour them. It's nice 
for sharing files between devs in the office, and it *is* safe.

No, it's not.

The whole point with --strict is that it shouldn't DWIM.  DWIMming is 
*NOT* safe if the data has previously passed through a security screen.

Don't DWIM in strict mode, ever.  If you do, you create security holes. 
  If not immediately, then later.

	-hpa

Andreas Ericsson wrote:

H. Peter Anvin wrote:

quoted

The whole point with --strict is that it shouldn't DWIM.  DWIMming is 
*NOT* safe if the data has previously passed through a security screen.

But it hasn't at this point. The security scan is done afterwards, when 
the canonical path is compared against the whitelist which, in strict 
mode, only matches if it matches exactly.

But anyways, how about doing

    enter_repo(path, 2)

from the daemon to make enter_repo() do the chdir(".git")?

... while preventing the later call from git-upload-pack from doing so.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

H. Peter Anvin wrote:

The whole point with --strict is that it shouldn't DWIM.  DWIMming is 
*NOT* safe if the data has previously passed through a security screen.

But it hasn't at this point. The security scan is done afterwards, when 
the canonical path is compared against the whitelist which, in strict 
mode, only matches if it matches exactly.

But anyways, how about doing

	enter_repo(path, 2)

from the daemon to make enter_repo() do the chdir(".git")?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231