@matheusbsbs / matheus-bagliano / commits / 20da61f25f

Git.pm: trust rev-parse to find bare repositories

When initializing a repository object, we run "git rev-parse --git-dir" to let the C version of Git find the correct directory. But curiously, if this fails we don't automatically say "not a git repository". Instead, we do our own pure-perl check to see if we're in a bare repository. This makes little sense, as rev-parse will report both bare and non-bare directories. This logic comes from d5c7721d58 (Git.pm: Add support for subdirectories inside of working copies, 2006-06-24), but I don't see any reason given why we can't just rely on rev-parse. Worse, because we treat any non-error response from rev-parse as a non-bare repository, we'll erroneously set the object's WorkingCopy, even in a bare repository. But it gets worse. Since 8959555cee (setup_git_directory(): add an owner check for the top-level directory, 2022-03-02), it's actively wrong (and dangerous). The perl code doesn't implement the same ownership checks. And worse, after "finding" the bare repository, it sets GIT_DIR in the environment, which tells any subsequent Git commands that we've confirmed the directory is OK, and to trust us. I.e., it re-opens the vulnerability plugged by 8959555cee when using Git.pm's repository discovery code. We can fix this by just relying on rev-parse to tell us when we're not in a repository, which fixes the vulnerability. Furthermore, we'll ask its --is-bare-repository function to tell us if we're bare or not, and rely on that. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Oct 22, 2022 at 18:08 UTC 20da61f25f8f61a2b581b60f8820ad6116f88e6f

3 files changed +32 -20

perl/Git.pm

+16 -20

@@ -177,16 +177,27 @@ sub repository {

177 -d $opts{Directory} or throw Error::Simple("Directory not found: $opts{Directory} $!");

178

179 my $search = Git->repository(WorkingCopy => $opts{Directory});

180 - my $dir;

180 +

181 + # This rev-parse will throw an exception if we're not in a

182 + # repository, which is what we want, but it's kind of noisy.

183 + # Ideally we'd capture stderr and relay it, but doing so is

184 + # awkward without depending on it fitting in a pipe buffer. So

185 + # we just reproduce a plausible error message ourselves.

186 + my $out;

187 try {

182 - $dir = $search->command_oneline(['rev-parse', '--git-dir'],

183 - STDERR => 0);

188 + # Note that "--is-bare-repository" must come first, as

189 + # --git-dir output could contain newlines.

190 + $out = $search->command([qw(rev-parse --is-bare-repository --git-dir)],

191 + STDERR => 0);

192 } catch Git::Error::Command with {

185 - $dir = undef;

193 + throw Error::Simple("fatal: not a git repository: $opts{Directory}");

194 };

195

196 + chomp $out;

197 + my ($bare, $dir) = split /\n/, $out, 2;

198 +

199 require Cwd;

189 - if ($dir) {

200 + if ($bare ne 'true') {

201 require File::Spec;

202 File::Spec->file_name_is_absolute($dir) or $dir = $opts{Directory} . '/' . $dir;

203 $opts{Repository} = Cwd::abs_path($dir);

@@ -204,21 +215,6 @@ sub repository {

215 $opts{WorkingSubdir} = $prefix;

216

217 } else {

207 - # A bare repository? Let's see...

208 - $dir = $opts{Directory};

209 -

210 - unless (-d "$dir/refs" and -d "$dir/objects" and -e "$dir/HEAD") {

211 - # Mimic git-rev-parse --git-dir error message:

212 - throw Error::Simple("fatal: Not a git repository: $dir");

213 - }

214 - my $search = Git->repository(Repository => $dir);

215 - try {

216 - $search->command('symbolic-ref', 'HEAD');

217 - } catch Git::Error::Command with {

218 - # Mimic git-rev-parse --git-dir error message:

219 - throw Error::Simple("fatal: Not a git repository: $dir");

220 - };

221 -

218 $opts{Repository} = Cwd::abs_path($dir);

219 }

220

t/t9700-perl-git.sh

@@ -45,6 +45,10 @@ test_expect_success \

45 git config --add test.pathmulti bar

46 '

48 +test_expect_success 'set up bare repository' '

49 + git init --bare bare.git

50 +'

51 +

52 test_expect_success 'use t9700/test.pl to test Git.pm' '

53 "$PERL_PATH" "$TEST_DIRECTORY"/t9700/test.pl 2>stderr &&

54 test_must_be_empty stderr

t/t9700/test.pl

+12

@@ -30,6 +30,18 @@ BEGIN { use_ok('Git') }

30 # set up

31 our $abs_repo_dir = cwd();

32 ok(our $r = Git->repository(Directory => "."), "open repository");

33 +{

34 + local $ENV{GIT_TEST_ASSUME_DIFFERENT_OWNER} = 1;

35 + my $failed;

36 +

37 + $failed = eval { Git->repository(Directory => $abs_repo_dir) };

38 + ok(!$failed, "reject unsafe non-bare repository");

39 + like($@, qr/not a git repository/i, "unsafe error message");

40 +

41 + $failed = eval { Git->repository(Directory => "$abs_repo_dir/bare.git") };

42 + ok(!$failed, "reject unsafe bare repository");

43 + like($@, qr/not a git repository/i, "unsafe error message");

44 +}

46 # config

47 is($r->config("test.string"), "value", "config scalar: string");