[BUG] remote.c/match_explicit() ... NULL pointer dereferenciation (git 1.5.3.4)

[BUG] remote.c/match_explicit() ... NULL pointer dereferenciation (git 1.5.3.4)

[Melchior FRANZ]

Hi,

I'm mucking around with git while implementing a simple, flat
CVS gateway. For tests I created a local remote clone via
"git remote add -f -t master -m master origin /local/path"
(as described on the git-remote man-page). When running a
(wrong) command like  "git push origin foo", whereby "foo"
is nowhere defined in the refspec list:

  [remote "origin"]
          url = /local/path
          fetch = +refs/heads/master:refs/remotes/origin/master
          push = +master:refs/heads/sync

then git-send-pack segfaults in remote.c/count_refspec_match
in the strlen() function, because "pattern" contains garbage.


And this is because in match_explicit() we have these lines:

        if (!matched_src)
                errs = 1;

        if (dst_value == NULL)
                dst_value = matched_src->name;
                                           <<- gdb prints from here


and with the unknown refspec "foo" both dst_value and matched_src
are zero:

  (gdb) print dst_value
  $1 = 0x0

  (gdb) print *rs
  $2 = {
    force = 0,
    pattern = 0,
    src = 0x808d680 "foo",
    dst = 0x0
  }

  (gdb) print matched_src
  $3 = (struct ref *) 0x0

  (gdb) print dst_value
  $4 = 0x34 <Address 0x34 out of bounds>


No idea, why the NULL-pointer dereferenciation doesn't segfault
right away, but assigns 0x34 to dst_value. Compiler bug?

m.



Spec:
  Linux 2.6.23.1  x86/P4
  gcc 4.2.1 (SUSE Linux)   (openSuSE 10.3)
  libc 2.6.1 (20070803)
  git 1.5.3.4  (compiled with -g -O0)

Re: [BUG] remote.c/match_explicit() ... NULL pointer dereferenciation (git 1.5.3.4)

[Junio C Hamano]

Subject: Prevent send-pack from segfaulting (backport from 'master')

4491e62ae932d5774f628d1bd3be663c11058a73 (Prevent send-pack from
segfaulting when a branch doesn't match) 

If we can't find a source match, and we have no destination, we
need to abort the match function early before we try to match
the destination against the remote.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

  Thanks.  Shawn fixed it on the 'master' side but 'maint' is
  still using the old code.

 remote.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/remote.c b/remote.c
index cdbbdcb..9a88917 100644
--- a/remote.c
+++ b/remote.c
@@ -504,8 +504,11 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	if (!matched_src)
 		errs = 1;
 
-	if (dst_value == NULL)
+	if (!dst_value) {
+		if (!matched_src)
+			return errs;
 		dst_value = matched_src->name;
+	}
 
 	switch (count_refspec_match(dst_value, dst, &matched_dst)) {
 	case 1: