[PATCH] gitweb: Harden parse_commit and parse_commits

[Jakub Narebski <jnareb@gmail.com>]

On Tue, 7 Feb 2012, Jakub Narebski wrote:
> On Mon, 6 Feb 2012, rajesh boyapati wrote:
[...]
> > Then, I restarted gerrit server to take changes.
> > Now the error log of gerrit shows:
> 
> > [2012-02-06 11:21:46,726] ERROR
> > com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision
> > 'HEAD'
> > [2012-02-06 11:21:49,167] ERROR
> > com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: [Mon Feb  6 11:21:49
> > 2012] gitweb.cgi: Use of uninitialized value $commit_id in open at
> > /usr/lib/cgi-bin/gitweb.cgi line 2817.
> > [2012-02-06 11:21:49,169] ERROR
> > com.google.gerrit.httpd.gitweb.GitWebServlet : CGI: fatal: bad revision ''
> [the same errors repeated few times]
> 
> > <<<<<<<<<<<<<<<<
> > Previously, there is a error showing at line 4720. Now, with this patch,
> > that error has gone.
> 
> As I said I was able to find a fix only for part of the issue.  
> Unfortunately I was not able to reproduce this error in this form.
> Note that the error location doesn't help much, because it is more
> interesting for find which callers of parse_commits() pass undefined
> $commit_id.
> 
> I can try to harden parse_commits() against bogus parameters; maybe
> this would help.

Does the following patch help, and does it fix the issue?

(Nb. you can try to simply change filename, and apply it with fuzz
against index.cgi file).
-- >8 -- ----- ----- ----- ----- ----- -- >8 --
From: Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH] gitweb: Harden parse_commit and parse_commits

Gitweb has problems and gives errors when repository it shows is on
unborn branch (HEAD doesn't point to a valid commit), but there exist
other branches.

One of errors that shows in gitweb logs is undefined $commit_id in
parse_commits() subroutine.  Therefore we harden both parse_commit()
and parse_commits() against undefined $commit_id, and against no
output from git-rev-list because HEAD doesn't point to a commit.

Reported-by: rajesh boyapati <boyapatisrajesh@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index f9535eb..1181aeb 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -3334,6 +3334,8 @@ sub parse_commit {
 	my ($commit_id) = @_;
 	my %co;
 
+	return unless defined $commit_id;
+
 	local $/ = "\0";
 
 	open my $fd, "-|", git_cmd(), "rev-list",
@@ -3343,7 +3345,9 @@ sub parse_commit {
 		$commit_id,
 		"--",
 		or die_error(500, "Open git-rev-list failed");
-	%co = parse_commit_text(<$fd>, 1);
+	my $commit_text = <$fd>;
+	%co = parse_commit_text($commit_text, 1)
+		if defined $commit_text;
 	close $fd;
 
 	return %co;
@@ -3353,6 +3357,7 @@ sub parse_commits {
 	my ($commit_id, $maxcount, $skip, $filename, @args) = @_;
 	my @cos;
 
+	return unless defined $commit_id;
 	$maxcount ||= 1;
 	$skip ||= 0;
 
-- 
1.7.9