* [PATCH 0/4] Fix minor warnings reported by icc
From: Ramkumar Ramachandra @ 2011-11-15 16:59 UTC (permalink / raw)
To: Git List; +Cc: Thomas Rast
Hi,
Thomas Rast tried compiling Git with Intel's C compiler (icc) recently
and the results can be found here [1]. Most of them don't amount to
anything, are probably not worth fixing. Here are fixes to a few that
caught my eye.
Thanks.
[1]: https://gist.github.com/1367335
-- Ram
Ramkumar Ramachandra (4):
http: remove unused function hex()
convert: don't mix enum with int
ll-merge: initialize default_opts const
sha1_file: don't mix enum with int
convert.c | 6 +++---
http.c | 8 --------
ll-merge.c | 2 +-
sha1_file.c | 2 +-
4 files changed, 5 insertions(+), 13 deletions(-)
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply
* [PATCH 2/4] convert: don't mix enum with int
From: Ramkumar Ramachandra @ 2011-11-15 16:59 UTC (permalink / raw)
To: Git List; +Cc: Thomas Rast
In-Reply-To: <1321376379-31750-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
convert.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/convert.c b/convert.c
index 3bb5a4d..038b0be 100644
--- a/convert.c
+++ b/convert.c
@@ -641,7 +641,7 @@ static int ident_to_worktree(const char *path, const char *src, size_t len,
return 1;
}
-static int git_path_check_crlf(const char *path, struct git_attr_check *check)
+static enum crlf_action git_path_check_crlf(const char *path, struct git_attr_check *check)
{
const char *value = check->value;
@@ -658,7 +658,7 @@ static int git_path_check_crlf(const char *path, struct git_attr_check *check)
return CRLF_GUESS;
}
-static int git_path_check_eol(const char *path, struct git_attr_check *check)
+static enum crlf_action git_path_check_eol(const char *path, struct git_attr_check *check)
{
const char *value = check->value;
@@ -811,7 +811,7 @@ int renormalize_buffer(const char *path, const char *src, size_t len, struct str
src = dst->buf;
len = dst->len;
}
- return ret | convert_to_git(path, src, len, dst, 0);
+ return ret | convert_to_git(path, src, len, dst, SAFE_CRLF_FALSE);
}
/*****************************************************************
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 1/4] http: remove unused function hex()
From: Ramkumar Ramachandra @ 2011-11-15 16:59 UTC (permalink / raw)
To: Git List; +Cc: Thomas Rast
In-Reply-To: <1321376379-31750-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
http.c | 8 --------
1 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/http.c b/http.c
index 008ad72..e6c7597 100644
--- a/http.c
+++ b/http.c
@@ -747,14 +747,6 @@ static inline int needs_quote(int ch)
return 1;
}
-static inline int hex(int v)
-{
- if (v < 10)
- return '0' + v;
- else
- return 'A' + v - 10;
-}
-
static char *quote_ref_url(const char *base, const char *ref)
{
struct strbuf buf = STRBUF_INIT;
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 3/4] ll-merge: initialize default_opts const
From: Ramkumar Ramachandra @ 2011-11-15 16:59 UTC (permalink / raw)
To: Git List; +Cc: Thomas Rast
In-Reply-To: <1321376379-31750-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
ll-merge.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/ll-merge.c b/ll-merge.c
index da59738..205aed3 100644
--- a/ll-merge.c
+++ b/ll-merge.c
@@ -351,7 +351,7 @@ int ll_merge(mmbuffer_t *result_buf,
const struct ll_merge_options *opts)
{
static struct git_attr_check check[2];
- static const struct ll_merge_options default_opts;
+ static const struct ll_merge_options default_opts = {0, 0, 0, 0};
const char *ll_driver_name = NULL;
int marker_size = DEFAULT_CONFLICT_MARKER_SIZE;
const struct ll_merge_driver *driver;
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 4/4] sha1_file: don't mix enum with int
From: Ramkumar Ramachandra @ 2011-11-15 16:59 UTC (permalink / raw)
To: Git List; +Cc: Thomas Rast
In-Reply-To: <1321376379-31750-1-git-send-email-artagnon@gmail.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
sha1_file.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sha1_file.c b/sha1_file.c
index 27f3b9b..869852b 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2616,7 +2616,7 @@ static int index_mem(unsigned char *sha1, void *buf, size_t size,
if ((type == OBJ_BLOB) && path) {
struct strbuf nbuf = STRBUF_INIT;
if (convert_to_git(path, buf, size, &nbuf,
- write_object ? safe_crlf : 0)) {
+ write_object ? safe_crlf : SAFE_CRLF_FALSE)) {
buf = strbuf_detach(&nbuf, &size);
re_allocated = 1;
}
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* [PATCH 5/4] git-compat-util: don't assume value for undefined variable
From: Ramkumar Ramachandra @ 2011-11-15 17:31 UTC (permalink / raw)
To: Git List; +Cc: Thomas Rast
In-Reply-To: <1321376379-31750-1-git-send-email-artagnon@gmail.com>
Suggested-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
And here's another probably worth fixing.
git-compat-util.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/git-compat-util.h b/git-compat-util.h
index 5ef8ff7..8b4dd5c 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -219,7 +219,7 @@ extern char *gitbasename(char *);
#define find_last_dir_sep(path) strrchr(path, '/')
#endif
-#if __HP_cc >= 61000
+#if defined(__HP_cc) && (__HP_cc >= 61000)
#define NORETURN __attribute__((noreturn))
#define NORETURN_PTR
#elif defined(__GNUC__) && !defined(NO_NORETURN)
--
1.7.6.351.gb35ac.dirty
^ permalink raw reply related
* Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork
From: Jeff King @ 2011-11-15 17:37 UTC (permalink / raw)
To: Thomas Rast
Cc: Franck Bui-Huu, Erik Faye-Lund, git, gitster, j6t, rene.scharfe
In-Reply-To: <201111151311.46832.trast@student.ethz.ch>
On Tue, Nov 15, 2011 at 01:11:46PM +0100, Thomas Rast wrote:
> But after a closer look I think this patch just prodded it enough to
> unearth long-existing undefined behaviour: prepare_argv() summarizes
> to something like
>
> static void prepare_argv(const char **sent_argv, const char **argv)
> {
> char *p, buf[4096];
>
> for (p = buf;;) {
> len = packet_read_line(0, p, (buf + sizeof buf) - p);
> /* ... p always points into buf ... */
> sent_argv[sent_argc++] = p;
> p += len;
> *p++ = 0;
> }
> sent_argv[sent_argc] = NULL;
> }
>
> The code appears to have looked like this ever since the addition of
> that file back in 39345a2 (Add git-upload-archive, 2006-09-07). So
> the elements of sent_argv have apparently always pointed into the
> stack-allocated 'buf'.
Oh, yikes. That is definitely the problem, but it does come from
c09cd77e. The prepare_argv function used to be "run_upload_archive", and
it would prepare argv on the stack, call into write_archive with it, and
then return; nobody else cares about the value afterwards.
Erik's patch converts it into a function that writes the new argv into a
parameter and returns, and the now-invalid stack-allocated memory is
used by the calling function.
> A quick band-aid would be to heap-allocate it instead:
That works. An even shorter band-aid is to mark it as "static".
I think the code would be more readable if it just used the new
argv_array.
Junio, this bug is in 1.7.8-rc*. Do you want the one-liner fix for the
release, or the nicer fix?
-Peff
^ permalink raw reply
* Re: input director not compatible with git right-click
From: Carlos Martín Nieto @ 2011-11-15 17:38 UTC (permalink / raw)
To: Sitaram Chamarty; +Cc: Eric, git
In-Reply-To: <CAMK1S_jWcLQTqzqQcAMk8PjZ4ir7Y7a8QY=JvmX2qbQnzJO4ew@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1348 bytes --]
On Tue, Nov 15, 2011 at 09:39:25PM +0530, Sitaram Chamarty wrote:
> On Mon, Nov 14, 2011 at 8:10 PM, Carlos Martín Nieto <cmn@elego.de> wrote:
> > On Sun, Nov 13, 2011 at 04:34:26PM +0000, Eric wrote:
> >> Hi,
> >>
> >> New in Git use, I use it do dev on window some administrative script. I use as
> >> well Input director to share keyboard and mouse on multiple computer.
> >
> > Do you mean you're using it on the Windows OS?
> >
> >>
> >> when I right-clicked on an item, it works when input director is disabled. If
> >
> > Right-click on what? git doesn't have a graphical interface. If you're
> > using a graphical front-end to git, you should send them a bug report.
>
> git comes with 3 perfectly cromulent graphical programs, and one of
> them is indispensable.
I guess we have diferent ideas of where "git" ends and other stuff
starts. gitk, git-gui and what is the last one?
>
> The real reason the original question is not meaningful here is that
> -- if he hadn't mentioned "share keyboard and mouse on multiple
> computer" most people would have no clue what the heck it was, and
> obscure software does... obscure things, so it should be reported to
> them. not here.
>
> --
> Sitaram
>
--
Carlos Martín Nieto | http://cmartin.tk
"¿Cómo voy a decir bobadas si soy mudo?" -- CACHAI
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork
From: Erik Faye-Lund @ 2011-11-15 17:44 UTC (permalink / raw)
To: Jeff King; +Cc: Thomas Rast, Franck Bui-Huu, git, gitster, j6t, rene.scharfe
In-Reply-To: <20111115173715.GA4478@sigill.intra.peff.net>
On Tue, Nov 15, 2011 at 6:37 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Nov 15, 2011 at 01:11:46PM +0100, Thomas Rast wrote:
>
>> But after a closer look I think this patch just prodded it enough to
>> unearth long-existing undefined behaviour: prepare_argv() summarizes
>> to something like
>>
>> static void prepare_argv(const char **sent_argv, const char **argv)
>> {
>> char *p, buf[4096];
>>
>> for (p = buf;;) {
>> len = packet_read_line(0, p, (buf + sizeof buf) - p);
>> /* ... p always points into buf ... */
>> sent_argv[sent_argc++] = p;
>> p += len;
>> *p++ = 0;
>> }
>> sent_argv[sent_argc] = NULL;
>> }
>>
>> The code appears to have looked like this ever since the addition of
>> that file back in 39345a2 (Add git-upload-archive, 2006-09-07). So
>> the elements of sent_argv have apparently always pointed into the
>> stack-allocated 'buf'.
>
> Oh, yikes. That is definitely the problem, but it does come from
> c09cd77e. The prepare_argv function used to be "run_upload_archive", and
> it would prepare argv on the stack, call into write_archive with it, and
> then return; nobody else cares about the value afterwards.
>
> Erik's patch converts it into a function that writes the new argv into a
> parameter and returns, and the now-invalid stack-allocated memory is
> used by the calling function.
>
Outch. Thanks for spotting.
>> A quick band-aid would be to heap-allocate it instead:
>
> That works. An even shorter band-aid is to mark it as "static".
Hmm, I seem to remember spotting it myself at some point and fixing it
by marking it as static. I guess I must have forgot to push it...
> I think the code would be more readable if it just used the new
> argv_array.
>
Oooh, nice. The whole argv_array slipped past me, I like it!
^ permalink raw reply
* Re: Git, Parrot, Perl6, Rakudo for G4 MAC
From: Hilco Wijbenga @ 2011-11-15 17:51 UTC (permalink / raw)
To: Greg; +Cc: git
In-Reply-To: <loom.20111115T112500-386@post.gmane.org>
On 15 November 2011 02:36, Greg <greggallen@gmail.com> wrote:
> Could someone please assist me in locating the resources to "GIT"
> this stuff going on a G4 MAC PPC? I
> keep getting weird bugs.
> Need me to be more explicit? Ok - it says gcc v3.3 isn't compatible, and a
> bunch of other sheet!
GCC 3.3 is from May 2003, I suggest you upgrade to a more recent GCC
(probably 4.6).
> I already have Perl5.10.1 working fine, and performing
> numerous marvelous tasks, so I (perhaps
> mistakenly) thought it would be an easy addition.
Perl 5.10.1 is from August 2009. Presumably less of a problem but,
again, you should probably upgrade to something more recent.
(Please note that I don't know the minimum requirements in this regard
for Git. It's just that your tools are old to ancient. :-) )
^ permalink raw reply
* Re: Git, Parrot, Perl6, Rakudo for G4 MAC
From: Jakub Narebski @ 2011-11-15 18:12 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Greg, git
In-Reply-To: <CAE1pOi0fqzoz+Af8PaSORuW45UqDAEmqtKW3w4qYH-275BdUzw@mail.gmail.com>
Hilco Wijbenga <hilco.wijbenga@gmail.com> writes:
> On 15 November 2011 02:36, Greg <greggallen@gmail.com> wrote:
> > Could someone please assist me in locating the resources to "GIT"
> > this stuff going on a G4 MAC PPC? I
> > keep getting weird bugs.
> > Need me to be more explicit? Ok - it says gcc v3.3 isn't compatible, and a
> > bunch of other sheet!
Yes, you need to be more explicit. Please copy'n'paste exact error
messages that you get compiling Git.
>
> GCC 3.3 is from May 2003, I suggest you upgrade to a more recent GCC
> (probably 4.6).
I don't have experience compiling Git for PPC, but on i386 I can
compile Git without problems with gcc 4.0.2.
> > I already have Perl5.10.1 working fine, and performing
> > numerous marvelous tasks, so I (perhaps
> > mistakenly) thought it would be an easy addition.
>
> Perl 5.10.1 is from August 2009. Presumably less of a problem but,
> again, you should probably upgrade to something more recent.
Git requires Perl version 5.8 at least, so you should not have any
problems here.
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork
From: Jeff King @ 2011-11-15 18:18 UTC (permalink / raw)
To: Erik Faye-Lund
Cc: Thomas Rast, Franck Bui-Huu, git, gitster, j6t, rene.scharfe
In-Reply-To: <CABPQNSbu9bb5UG1ihi2vGX5ZkpvjfjaXrT8bhDGZHb=n9RbjSg@mail.gmail.com>
On Tue, Nov 15, 2011 at 06:44:38PM +0100, Erik Faye-Lund wrote:
> > I think the code would be more readable if it just used the new
> > argv_array.
> >
> Oooh, nice. The whole argv_array slipped past me, I like it!
I don't think it existed when you started on this series. :)
-Peff
^ permalink raw reply
* Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork
From: Andreas Schwab @ 2011-11-15 18:53 UTC (permalink / raw)
To: Thomas Rast
Cc: Jeff King, Franck Bui-Huu, Erik Faye-Lund, git, gitster, j6t,
rene.scharfe
In-Reply-To: <201111151311.46832.trast@student.ethz.ch>
Thomas Rast <trast@student.ethz.ch> writes:
> But after a closer look I think this patch just prodded it enough to
> unearth long-existing undefined behaviour: prepare_argv() summarizes
> to something like
>
> static void prepare_argv(const char **sent_argv, const char **argv)
> {
> char *p, buf[4096];
>
> for (p = buf;;) {
> len = packet_read_line(0, p, (buf + sizeof buf) - p);
> /* ... p always points into buf ... */
> sent_argv[sent_argc++] = p;
> p += len;
> *p++ = 0;
> }
> sent_argv[sent_argc] = NULL;
> }
>
> The code appears to have looked like this ever since the addition of
> that file back in 39345a2 (Add git-upload-archive, 2006-09-07). So
> the elements of sent_argv have apparently always pointed into the
> stack-allocated 'buf'.
>
> (This correlates with the "Address 0x7feffe7d0 is not stack'd", even
> though it's pretty clearly an address into the stack.)
>
> A quick band-aid would be to heap-allocate it instead:
Or allocate it in the caller:
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index c57e8bd..f0f843e 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -18,11 +18,12 @@ static const char lostchild[] =
"git upload-archive: archiver process was lost";
#define MAX_ARGS (64)
+#define ARGV_BUF_SIZE 4096
-static void prepare_argv(const char **sent_argv, const char **argv)
+static void prepare_argv(const char **sent_argv, char *buf, const char **argv)
{
const char *arg_cmd = "argument ";
- char *p, buf[4096];
+ char *p;
int sent_argc;
int len;
@@ -32,7 +33,7 @@ static void prepare_argv(const char **sent_argv, const char **argv)
sent_argv[1] = "--remote-request";
for (p = buf;;) {
/* This will die if not enough free space in buf */
- len = packet_read_line(0, p, (buf + sizeof buf) - p);
+ len = packet_read_line(0, p, (buf + ARGV_BUF_SIZE) - p);
if (len == 0)
break; /* got a flush */
if (sent_argc > MAX_ARGS - 2)
@@ -85,6 +86,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
{
const char *sent_argv[MAX_ARGS];
struct child_process cld = { sent_argv };
+ char argv_buf[ARGV_BUF_SIZE];
cld.out = cld.err = -1;
cld.git_cmd = 1;
@@ -94,7 +96,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
if (!enter_repo(argv[1], 0))
die("'%s' does not appear to be a git repository", argv[1]);
- prepare_argv(sent_argv, argv);
+ prepare_argv(sent_argv, argv_buf, argv);
if (start_command(&cld)) {
int err = errno;
packet_write(1, "NACK fork failed on the remote side\n");
--
1.7.7.3
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply related
* Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork
From: Junio C Hamano @ 2011-11-15 18:59 UTC (permalink / raw)
To: Jeff King
Cc: Thomas Rast, Franck Bui-Huu, Erik Faye-Lund, git, gitster, j6t,
rene.scharfe
In-Reply-To: <20111115173715.GA4478@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> Junio, this bug is in 1.7.8-rc*. Do you want the one-liner fix for the
> release, or the nicer fix?
Let's just do "static" for now, if we know the array is large enough.
^ permalink raw reply
* Re: [PATCH] git-show-ref: fix escaping in asciidoc source
From: Junio C Hamano @ 2011-11-15 19:16 UTC (permalink / raw)
To: Michael Haggerty; +Cc: git
In-Reply-To: <4EC27328.3070309@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> Junio,
>
> Did this one fall through the cracks? I don't see it in your tree.
>
> Michael
Yeah, I was wondering if we can have a concise description in what context
any "^" must be spelled as {caret} and what other context "^" can be
spelled literally, and possibly which versions of AsciiDoc toolchain have
this issue [*1*]. Without a clear guideline, people may unknowingly use
literal "^" to new paragraphs, or perhaps worse yet, spell {caret} that
end up being shown literally.
Since I didn't find a clear pattern other than that "^" can and should be
literally given in a literal paragraph (i.e. an indented paragraph or
inside a listing/literal block that shows program examples), I was meaning
to ask you if you knew the rules better than I did, and I stopped there,
forgetting to follow through.
[Footnote]
*1* For example, http://schacon.github.com/git/git-show-ref.html indicates
that the description for "-d" does not seem to need your patch for the box
it was formatted on.
> On 10/19/2011 08:52 PM, mhagger@alum.mit.edu wrote:
>> From: Michael Haggerty <mhagger@alum.mit.edu>
>>
>> One of the "^" characters was not coming through in the man page.
>>
>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
>> ---
>> Documentation/git-show-ref.txt | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/git-show-ref.txt b/Documentation/git-show-ref.txt
>> index 3c45895..87f358d 100644
>> --- a/Documentation/git-show-ref.txt
>> +++ b/Documentation/git-show-ref.txt
>> @@ -44,7 +44,7 @@ OPTIONS
>> -d::
>> --dereference::
>>
>> - Dereference tags into object IDs as well. They will be shown with "^{}"
>> + Dereference tags into object IDs as well. They will be shown with "{caret}\{\}"
>> appended.
>>
>> -s::
>> @@ -75,7 +75,7 @@ OPTIONS
>> Make 'git show-ref' act as a filter that reads refs from stdin of the
>> form "^(?:<anything>\s)?<refname>(?:{backslash}{caret}\{\})?$"
>> and performs the following actions on each:
>> - (1) strip "^{}" at the end of line if any;
>> + (1) strip "{caret}\{\}" at the end of line if any;
>> (2) ignore if pattern is provided and does not head-match refname;
>> (3) warn if refname is not a well-formed refname and skip;
>> (4) ignore if refname is a ref that exists in the local repository;
^ permalink raw reply
* Re: [PATCH v4 3/3] upload-archive: use start_command instead of fork
From: Jeff King @ 2011-11-15 19:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Rast, Franck Bui-Huu, Erik Faye-Lund, git, j6t,
rene.scharfe
In-Reply-To: <7vobwdus7w.fsf@alter.siamese.dyndns.org>
On Tue, Nov 15, 2011 at 10:59:47AM -0800, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > Junio, this bug is in 1.7.8-rc*. Do you want the one-liner fix for the
> > release, or the nicer fix?
>
> Let's just do "static" for now, if we know the array is large enough.
OK, here it is.
I think it's correct, but I couldn't reproduce the valgrind failure
here. Thomas, can you double check that this also solves your problem?
-Peff
-- >8 --
Subject: [PATCH] upload-archive: don't return pointers to stack buffer
The prepare_argv function uses an internal stack-allocated
buffer to create the argv array that will be used to run
git-archive. Prior to c09cd77e, this was OK, as the
function passed the argv array to write_archive itself, and
the stack buffer was valid during its use.
Since c09cd77e, however, the function returns an argv array
with pointers pointing into the stack buffer. The calling
function then passes the result to start_command, which
tries to execve using pointers to a now-invalid buffer.
Fix it by making the buffer static, which is a quick and
simple fix, and works because we only run this function once
per process.
Credit for finding the bug and most of the analysis goes to
Thomas Rast.
Signed-off-by: Jeff King <peff@peff.net>
---
builtin/upload-archive.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index c57e8bd..f47c0f0 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -22,7 +22,8 @@
static void prepare_argv(const char **sent_argv, const char **argv)
{
const char *arg_cmd = "argument ";
- char *p, buf[4096];
+ char *p;
+ static char buf[4096];
int sent_argc;
int len;
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* Re: [PATCH] Fix "is_refname_available(): query only possibly-conflicting references"
From: Junio C Hamano @ 2011-11-15 19:19 UTC (permalink / raw)
To: Michael Haggerty
Cc: git, Jeff King, Drew Northup, Jakub Narebski, Heiko Voigt,
Johan Herland, Julian Phillips
In-Reply-To: <4EC29120.2020400@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
> If you have a preference for which patch series you would like to
> integrate in which order (and especially if you think that there are
> gaps that need to be filled), please let me know. It would be a lot
> less work to put them in the right order from the start rather than
> trying to keep them all synchronized with master and continually reroll
> them based on what you have merged so far.
I've re-read mh/ref-api-[23] a few times myself during this feature-freeze
period and found that the checks they enforce seemed to be sensible for
newly created refs. But I do not know if there are widespread mispractices
of using "wrong" refnames, created by either older versions of Git and
common third-party ones, that may make existing repository unusable
without first correcting them, and more importantly, I do not know if the
updated code is lenient enough to give users necessary escape hatches to
correct the existing problems. As we recently found out on the 'master'
front for your earlier topic, an updated check that is more strict and
saner than the older one is not necessarily an improvement, if it causes
pain to existing users to adjust to the new world order.
The output frm "git log --oneline --first-parent master..pu" should give
rough idea of what I have in mind. Obvious and nonintrusive clean-ups come
early, then features that are shown to be needed in the field and are with
user-facing design that are perfected come next to give them longer time
to fix potential issues in implemementation, followed by all the rest.
^ permalink raw reply
* [PATCH 1/2] upload-archive: drop extra argument to prepare_argv
From: Jeff King @ 2011-11-15 19:46 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Rast, Franck Bui-Huu, Erik Faye-Lund, git, j6t,
rene.scharfe
In-Reply-To: <20111115191832.GA16030@sigill.intra.peff.net>
We pass a "sent_argv" which is an out-parameter to hold the
argv that we were sent over the wire. But we also pass in
the "argv" we got on the command line, which is not used at
all. Drop this useless and confusing parameter.
The parameter was obsoleted by c09cd77e, which moved the
enter_repo function (which looked at argv[1]) out of
prepare_argv and into cmd_upload_archive.
Signed-off-by: Jeff King <peff@peff.net>
---
On Tue, Nov 15, 2011 at 02:18:32PM -0500, Jeff King wrote:
> > Let's just do "static" for now, if we know the array is large enough.
>
> OK, here it is.
And here's the other more invasive cleanup on top (patch 2 is the meaty
one).
builtin/upload-archive.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index f47c0f0..80575b9 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -19,7 +19,7 @@
#define MAX_ARGS (64)
-static void prepare_argv(const char **sent_argv, const char **argv)
+static void prepare_argv(const char **sent_argv)
{
const char *arg_cmd = "argument ";
char *p;
@@ -95,7 +95,7 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
if (!enter_repo(argv[1], 0))
die("'%s' does not appear to be a git repository", argv[1]);
- prepare_argv(sent_argv, argv);
+ prepare_argv(sent_argv);
if (start_command(&cld)) {
int err = errno;
packet_write(1, "NACK fork failed on the remote side\n");
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* [PATCH] upload-archive: use argv_array for sent parameters
From: Jeff King @ 2011-11-15 19:49 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Rast, Franck Bui-Huu, Erik Faye-Lund, git, j6t,
rene.scharfe
In-Reply-To: <20111115191832.GA16030@sigill.intra.peff.net>
The existing prepare_argv uses a fixed-size static buffer to
hold all of the arguments, and then puts pointers into the
buffer into a fixed-size array. Using argv_array gets rid of
all of the manual bookkeeping and makes the code more
readable.
It also lifts the static limits on the size of the array.
This is convenient, but is perhaps a security regression, as
a malicious client can now ask us to create arbitrary-length
argv arrays in memory.
Signed-off-by: Jeff King <peff@peff.net>
---
I think the code is way more readable and obvious. Do we care about the
potential DoS? If so, we can still cap the number of arguments we'll
accept. In practice, we never send more than a few.
builtin/upload-archive.c | 45 +++++++++++++++++----------------------------
1 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c
index 80575b9..3830310 100644
--- a/builtin/upload-archive.c
+++ b/builtin/upload-archive.c
@@ -7,6 +7,7 @@
#include "pkt-line.h"
#include "sideband.h"
#include "run-command.h"
+#include "argv-array.h"
static const char upload_archive_usage[] =
"git upload-archive <repo>";
@@ -17,42 +18,28 @@
static const char lostchild[] =
"git upload-archive: archiver process was lost";
-#define MAX_ARGS (64)
-
-static void prepare_argv(const char **sent_argv)
+static void prepare_argv(struct argv_array *out)
{
const char *arg_cmd = "argument ";
- char *p;
- static char buf[4096];
- int sent_argc;
- int len;
- /* put received options in sent_argv[] */
- sent_argc = 2;
- sent_argv[0] = "archive";
- sent_argv[1] = "--remote-request";
- for (p = buf;;) {
+ argv_array_push(out, "archive");
+ argv_array_push(out, "--remote-request");
+ while (1) {
+ char buf[4096];
+ int len;
/* This will die if not enough free space in buf */
- len = packet_read_line(0, p, (buf + sizeof buf) - p);
+ len = packet_read_line(0, buf, sizeof(buf));
if (len == 0)
break; /* got a flush */
- if (sent_argc > MAX_ARGS - 2)
- die("Too many options (>%d)", MAX_ARGS - 2);
- if (p[len-1] == '\n') {
- p[--len] = 0;
- }
+ if (buf[len-1] == '\n')
+ buf[--len] = 0;
if (len < strlen(arg_cmd) ||
- strncmp(arg_cmd, p, strlen(arg_cmd)))
+ strncmp(arg_cmd, buf, strlen(arg_cmd)))
die("'argument' token or flush expected");
- len -= strlen(arg_cmd);
- memmove(p, p + strlen(arg_cmd), len);
- sent_argv[sent_argc++] = p;
- p += len;
- *p++ = 0;
+ argv_array_push(out, buf + strlen(arg_cmd));
}
- sent_argv[sent_argc] = NULL;
}
__attribute__((format (printf, 1, 2)))
@@ -84,8 +71,8 @@ static ssize_t process_input(int child_fd, int band)
int cmd_upload_archive(int argc, const char **argv, const char *prefix)
{
- const char *sent_argv[MAX_ARGS];
- struct child_process cld = { sent_argv };
+ struct argv_array sent_argv = ARGV_ARRAY_INIT;
+ struct child_process cld = {0};
cld.out = cld.err = -1;
cld.git_cmd = 1;
@@ -95,7 +82,8 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
if (!enter_repo(argv[1], 0))
die("'%s' does not appear to be a git repository", argv[1]);
- prepare_argv(sent_argv);
+ prepare_argv(&sent_argv);
+ cld.argv = sent_argv.argv;
if (start_command(&cld)) {
int err = errno;
packet_write(1, "NACK fork failed on the remote side\n");
@@ -138,5 +126,6 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
packet_flush(1);
break;
}
+ argv_array_clear(&sent_argv);
return 0;
}
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* [PATCH v2] Add built-in diff patterns for MATLAB code
From: Gustaf Hendeby @ 2011-11-15 20:15 UTC (permalink / raw)
To: Thomas Rast; +Cc: Git List, Junio C Hamano, Gustaf Hendeby
In-Reply-To: <201111151414.34141.trast@student.ethz.ch>
MATLAB is often used in industry and academia for scientific
computations motivating it being included as a build in pattern.
Signed-off-by: Gustaf Hendeby <hendeby@isy.liu.se>
---
This version of this commit adds the missing patterns to make the
.-operators words and adds these to the test. Lots of thanks Thomas
for keeping your eyes open and catching this.
/Gustaf
Documentation/gitattributes.txt | 2 ++
t/t4018-diff-funcname.sh | 2 +-
t/t4034-diff-words.sh | 1 +
t/t4034/matlab/expect | 14 ++++++++++++++
t/t4034/matlab/post | 9 +++++++++
t/t4034/matlab/pre | 9 +++++++++
userdiff.c | 3 +++
7 files changed, 39 insertions(+), 1 deletions(-)
create mode 100644 t/t4034/matlab/expect
create mode 100644 t/t4034/matlab/post
create mode 100644 t/t4034/matlab/pre
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index 25e46ae..a85b187 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -500,6 +500,8 @@ patterns are available:
- `java` suitable for source code in the Java language.
+- `matlab` suitable for source code in the MATLAB language.
+
- `objc` suitable for source code in the Objective-C language.
- `pascal` suitable for source code in the Pascal/Delphi language.
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index b68c56b..4bd2a1c 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -105,7 +105,7 @@ test_expect_funcname () {
grep "^@@.*@@ $1" diff
}
-for p in bibtex cpp csharp fortran html java objc pascal perl php python ruby tex
+for p in bibtex cpp csharp fortran html java matlab objc pascal perl php python ruby tex
do
test_expect_success "builtin $p pattern compiles" '
echo "*.java diff=$p" >.gitattributes &&
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index c374aa4..6f1e5a2 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -299,6 +299,7 @@ test_language_driver csharp
test_language_driver fortran
test_language_driver html
test_language_driver java
+test_language_driver matlab
test_language_driver objc
test_language_driver pascal
test_language_driver perl
diff --git a/t/t4034/matlab/expect b/t/t4034/matlab/expect
new file mode 100644
index 0000000..72cf3e9
--- /dev/null
+++ b/t/t4034/matlab/expect
@@ -0,0 +1,14 @@
+<BOLD>diff --git a/pre b/post<RESET>
+<BOLD>index dc204db..70e05f0 100644<RESET>
+<BOLD>--- a/pre<RESET>
+<BOLD>+++ b/post<RESET>
+<CYAN>@@ -1,9 +1,9 @@<RESET>
+(<RED>1<RESET><GREEN>0<RESET>) (<RED>-1e10<RESET><GREEN>-0e10<RESET>) '<RED>b<RESET><GREEN>y<RESET>';
+[<RED>a<RESET><GREEN>x<RESET>] {<RED>a<RESET><GREEN>x<RESET>} <RED>a<RESET><GREEN>x<RESET>.<RED>b<RESET><GREEN>y<RESET>;
+~<RED>a<RESET><GREEN>x<RESET>;
+<RED>a<RESET><GREEN>x<RESET>*<RED>b a<RESET><GREEN>y x<RESET>.*<RED>b a<RESET><GREEN>y x<RESET>/<RED>b a<RESET><GREEN>y x<RESET>./<RED>b a<RESET><GREEN>y x<RESET>^<RED>b a<RESET><GREEN>y x<RESET>.^<RED>b a<RESET><GREEN>y x<RESET>.\<RED>b a<RESET><GREEN>y x<RESET>.';
+<RED>a<RESET><GREEN>x<RESET>+<RED>b a<RESET><GREEN>y x<RESET>-<RED>b<RESET><GREEN>y<RESET>;
+<RED>a<RESET><GREEN>x<RESET>&<RED>b a<RESET><GREEN>y x<RESET>&&<RED>b a<RESET><GREEN>y x<RESET>|<RED>b a<RESET><GREEN>y x<RESET>||<RED>b<RESET><GREEN>y<RESET>;
+<RED>a<RESET><GREEN>x<RESET><<RED>b a<RESET><GREEN>y x<RESET><=<RED>b a<RESET><GREEN>y x<RESET>><RED>b a<RESET><GREEN>y x<RESET>>=<RED>b<RESET><GREEN>y<RESET>;
+<RED>a<RESET><GREEN>x<RESET>==<RED>b a<RESET><GREEN>y x<RESET>~=<RED>b<RESET><GREEN>y<RESET>;
+<RED>a<RESET><GREEN>x<RESET>,<RED>b<RESET><GREEN>y<RESET>;
diff --git a/t/t4034/matlab/post b/t/t4034/matlab/post
new file mode 100644
index 0000000..70e05f0
--- /dev/null
+++ b/t/t4034/matlab/post
@@ -0,0 +1,9 @@
+(0) (-0e10) 'y';
+[x] {x} x.y;
+~x;
+x*y x.*y x/y x./y x^y x.^y x.\y x.';
+x+y x-y;
+x&y x&&y x|y x||y;
+x<y x<=y x>y x>=y;
+x==y x~=y;
+x,y;
diff --git a/t/t4034/matlab/pre b/t/t4034/matlab/pre
new file mode 100644
index 0000000..dc204db
--- /dev/null
+++ b/t/t4034/matlab/pre
@@ -0,0 +1,9 @@
+(1) (-1e10) 'b';
+[a] {a} a.b;
+~a;
+a*b a.*b a/b a./b a^b a.^b a.\b a.';
+a+b a-b;
+a&b a&&b a|b a||b;
+a<b a<=b a>b a>=b;
+a==b a~=b;
+a,b;
diff --git a/userdiff.c b/userdiff.c
index bf553ad..7c983c1 100644
--- a/userdiff.c
+++ b/userdiff.c
@@ -37,6 +37,9 @@ PATTERNS("java",
"|[-+0-9.e]+[fFlL]?|0[xXbB]?[0-9a-fA-F]+[lL]?"
"|[-+*/<>%&^|=!]="
"|--|\\+\\+|<<=?|>>>?=?|&&|\\|\\|"),
+PATTERNS("matlab",
+ "^[[:space:]]*((classdef|function)[[:space:]].*)$|^%%[[:space:]].*$",
+ "[a-zA-Z_][a-zA-Z0-9_]*|[-+0-9.e]+|[=~<>]=|\\.[*/\\^']|\\|\\||&&"),
PATTERNS("objc",
/* Negate C statements that can look like functions */
"!^[ \t]*(do|for|if|else|return|switch|while)\n"
--
1.7.8.rc2.209.geddc4
^ permalink raw reply related
* odd name-rev behavior?
From: Tim Walberg @ 2011-11-15 21:15 UTC (permalink / raw)
To: git
Never noticed this before - is this expected? It doesn't seem to
match documentation, in any case...
Basically, 'git name-rev --all' lists a lot of lines that are
marked as "undefined", and most, if not all, of the objects
represented are trees, not commits... I wouldn't have expected
name-rev to follow the link from a commit to a tree and try to
process that result...
> git --version
git version 1.7.7.3
> git name-rev --all | tail -10
ed858730bc0e40d5317543ca4a199f1902b1c545 master~35
ea6d0d16af77b357caab59bda3eff8efe78370c1 undefined
ea01449260d4ed49cc30ffff8fa840273e182195 undefined
f0a186fa05e9d4dd2de85ab537ac019a74b84736 undefined
f223486e4178e0b2d6645fa3d0478602a6aec548 undefined
f6f969d943f981057d9243d5fc2d8a79864cb542 master~19
f73dc68c32681939d2ce92378128889c7b6a8902 master~90
f739f050f0a20692e947604d0d5a464cdd5393aa master~109
faa7bf622b69c93f1f21eec0de3bd0ab6957b888 master~94
ff41d4390d3fabf1ba886749a51a20722b159fdd undefined
> git cat-file -t ff41d4
tree
>
--
+----------------------+
| Tim Walberg |
| 830 Carriage Dr. |
| Algonquin, IL 60102 |
| twalberg@comcast.net |
+----------------------+
^ permalink raw reply
* Re: [PATCH] upload-archive: use argv_array for sent parameters
From: Jeff King @ 2011-11-15 21:30 UTC (permalink / raw)
To: Junio C Hamano
Cc: Thomas Rast, Franck Bui-Huu, Erik Faye-Lund, git, j6t,
rene.scharfe
In-Reply-To: <20111115194958.GB19305@sigill.intra.peff.net>
On Tue, Nov 15, 2011 at 02:49:58PM -0500, Jeff King wrote:
> The existing prepare_argv uses a fixed-size static buffer to
> hold all of the arguments, and then puts pointers into the
> buffer into a fixed-size array. Using argv_array gets rid of
> all of the manual bookkeeping and makes the code more
> readable.
>
> It also lifts the static limits on the size of the array.
> This is convenient, but is perhaps a security regression, as
> a malicious client can now ask us to create arbitrary-length
> argv arrays in memory.
>
> Signed-off-by: Jeff King <peff@peff.net>
Sorry, I failed to mark the subject properly. If it was not obvious,
this is patch 2/2 of the cleanup.
-Peff
^ permalink raw reply
* [PATCH 0/2] upload-archive security issues
From: Jeff King @ 2011-11-15 21:42 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erik Faye-Lund, git
[Note to readers who haven't been following the recent thread on
upload-archive bugs: these security issues are in c09cd77e, which has
not actually been released. So this is "security problems, and we need
fixes before this ships in 1.7.8" and not "OMG your git site is 0wned"].
Looking at Erik's c09cd77e again, there are some serious security
problems, in that we are too lenient with what gets passed to
git-archive, which is not hardened to accept random client arguments.
That lets a client do all sorts of nasty things like running arbitrary
code.
These patches fix it by making cmd_archive handle the remote-request
flag better. An alternative would be to pass only known-good options
through upload-archive. That might be more future-proof, but also
involves upload-archive knowing about the innards of write_archive and
its options. See also the comments in patch 2/2 for another alternative
fix.
[1/2]: archive: don't allow negation of --remote-request
[2/2]: archive: limit ourselves during remote requests
And yes, I feel like a moron for not noticing these problems during my
initial review.
-Peff
^ permalink raw reply
* [PATCH 1/2] archive: don't allow negation of --remote-request
From: Jeff King @ 2011-11-15 21:43 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erik Faye-Lund, git
In-Reply-To: <20111115214159.GA20457@sigill.intra.peff.net>
The remote-request flag is a security feature, telling the
spawned git-archive that certain formats should be turned
off. We always place it at the front of the command line
when serving a remote request. Of course, this doesn't do us
any good if the client can simply ask us politely to turn it
off.
This bug was introduced in c09cd77 (upload-archive: use
start_command instead of fork, 2011-10-24), but hasn't yet
been released.
Signed-off-by: Jeff King <peff@peff.net>
---
The other option would be recognizing and disallowing this when reading
arguments from the remote.
builtin/archive.c | 2 +-
t/t5000-tar-tree.sh | 12 ++++++++++++
2 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/builtin/archive.c b/builtin/archive.c
index e405566..fce20a1 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -97,7 +97,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
"path to the remote git-upload-archive command"),
{ OPTION_BOOLEAN, 0, "remote-request", &is_remote, NULL,
"indicate we are serving a remote request",
- PARSE_OPT_NOARG | PARSE_OPT_HIDDEN },
+ PARSE_OPT_NOARG | PARSE_OPT_HIDDEN | PARSE_OPT_NONEG },
OPT_END()
};
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 889842e..723b54e 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -305,6 +305,18 @@ test_expect_success 'only enabled filters are available remotely' '
test_cmp remote.bar config.bar
'
+# We have to hand-craft this, since the local "git archive" will
+# eat our "--no-remote-request" argument otherwise.
+test_expect_success 'malicious clients cannot un-remote themselves' '
+ {
+ echo "0021argument --no-remote-request" &&
+ echo "001eargument --format=tar.foo" &&
+ echo "0012argument HEAD" &&
+ printf "0000"
+ } >evil-request &&
+ test_must_fail git upload-archive . <evil-request >remote.tar.foo
+'
+
if $GZIP --version >/dev/null 2>&1; then
test_set_prereq GZIP
else
--
1.7.7.3.8.g38efa
^ permalink raw reply related
* [PATCH 2/2] archive: limit ourselves during remote requests
From: Jeff King @ 2011-11-15 21:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Erik Faye-Lund, git
In-Reply-To: <20111115214159.GA20457@sigill.intra.peff.net>
Originally, the call-chain of an upload-archive invocation
was like:
cmd_archive (on local machine)
run_remote_archiver (local)
cmd_upload_archive (on remote machine)
run_upload_archive (remote)
write_archive (remote)
And write_archive knew that it could be remotely invoked,
and didn't trust the arguments given to it when doing
anything security-critical.
Since c09cd77 (upload-archive: use start_command instead of
fork, 2011-10-24), we exec a git-archive subprocess, and the
call-chain is now:
cmd_archive (local)
run_remote_archiver (local)
cmd_upload_archive (remote)
cmd_archive (in a sub-process)
write_archive
The arbitrary arguments we get from the client are passed
through cmd_archive via the command-line of git-archive.
Unlike write_archive, cmd_archive was never taught not to
trust the remote arguments. Among the many horrible things a
malicious client could do were:
- accessing another repository as the user running on the
server, using "--remote"
- execute arbitrary code as the user running on the server
using "--remote --exec"
- overwrite arbitrary files using "--output"
This patch causes cmd_archive to respect the remote-request
flag immediately and chain to write_archive, ignoring any
other options.
Signed-off-by: Jeff King <peff@peff.net>
---
This is the minimal required to fix it.
I mentioned already the alternative of allowing through only known-good
arguments in upload-pack's prepare_argv. I don't like that because it
means we have to know about every option that write_archive is OK with.
I think a more sensible solution would be a new command, "git
upload-archive--remote", which is just a very stripped-down version of
git-archive (i.e., it _only_ calls write_archive). Or it could even be
spelled "git upload-archive --remote-request". But the point is that
git-archive never needed to worry about security before. We
shouldn't be polluting it with security code; we should be bypassing it
going to write_archive directly.
For the tests, checking each failure mode is perhaps overkill, but I
want to be double sure that this doesn't ever regress.
builtin/archive.c | 9 +++++++++
t/t5000-tar-tree.sh | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 0 deletions(-)
diff --git a/builtin/archive.c b/builtin/archive.c
index fce20a1..ee2fb54 100644
--- a/builtin/archive.c
+++ b/builtin/archive.c
@@ -104,6 +104,15 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
argc = parse_options(argc, argv, prefix, local_opts, NULL,
PARSE_OPT_KEEP_ALL);
+ /*
+ * We want to ignore all other parsed options in the remote case, as
+ * they come from an arbitrary client. Therefore we shouldn't do things
+ * like write files based on --output, or make new --remote
+ * connections.
+ */
+ if (is_remote)
+ return write_archive(argc, argv, prefix, 0, NULL, 1);
+
if (output)
create_output_file(output);
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index 723b54e..5679c79 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -317,6 +317,45 @@ test_expect_success 'malicious clients cannot un-remote themselves' '
test_must_fail git upload-archive . <evil-request >remote.tar.foo
'
+# Again, we have to hand-craft our malicious request. Since parsing
+# the output to determine that we did indeed get subrepo would be hard,
+# instead we use an easier test: try to get a branch only in the subrepo,
+# which must fail if our hack doesn't work.
+test_expect_success 'malicious clients cannot access repos via --remote' '
+ git init subrepo &&
+ (cd subrepo &&
+ test_commit subrepo &&
+ git branch only-in-subrepo
+ ) &&
+ {
+ echo "0021argument --remote=../subrepo"
+ echo "001dargument only-in-subrepo" &&
+ printf "0000"
+ } >evil-request &&
+ test_must_fail git upload-archive . <evil-request >evil-output
+'
+
+test_expect_success 'malicious clients cannot exec code via --remote' '
+ {
+ echo "0021argument --remote=../subrepo"
+ echo "0026argument --exec=echo foo >hax0red"
+ echo "0012argument HEAD" &&
+ printf "0000"
+ } >evil-request &&
+ test_might_fail git upload-archive . <evil-request >evil-output &&
+ test_path_is_missing .git/hax0red
+'
+
+test_expect_success 'malicious clients cannot trigger --output on server' '
+ {
+ echo "001dargument --output=p0wn3d"
+ echo "0012argument HEAD" &&
+ printf "0000"
+ } >evil-request &&
+ git upload-archive . <evil-request >remote.tar &&
+ test_path_is_missing .git/p0wn3d
+'
+
if $GZIP --version >/dev/null 2>&1; then
test_set_prereq GZIP
else
--
1.7.7.3.8.g38efa
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox