gitweb: Consolidate escaping/validation of query string

Consider:

http://repo.or.cz/?p=glibc-cvs.git;a=tree;h=2609cb0411389325f4ee2854cc7159756eb0671e;hb=2609cb0411389325f4ee2854cc7159756eb0671e

(click on the funny =__ify file)

We ought to handle anything in filenames and I actually see no reason why
we don't, modulo very little missing escaping that this patch hopefully
also fixes.

I have also made esc_param() escape [?=&;]. Not escaping [&;] was downright
buggy and [?=] just feels better escaped. ;-) YMMV.

Signed-off-by: Petr Baudis <pasky@suse.cz>
Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 3d06181229a089ad63d7a46744680a3b161a72f3..0693a833c173ab9218ea791b7678d399129a83cf 100755 (executable)
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -212,19 +212,9 @@ sub feature_pickaxe {
        }
 }
 
+# We have to handle those containing any characters:
 our $file_name = $cgi->param('f');
-if (defined $file_name) {
-       if (!validate_input($file_name)) {
-               die_error(undef, "Invalid file parameter");
-       }
-}
-
 our $file_parent = $cgi->param('fp');
-if (defined $file_parent) {
-       if (!validate_input($file_parent)) {
-               die_error(undef, "Invalid file parent parameter");
-       }
-}
 
 our $hash = $cgi->param('h');
 if (defined $hash) {
@@ -305,7 +295,7 @@ sub evaluate_path_info {
                        $action  ||= "blob_plain";
                }
                $hash_base ||= validate_input($refname);
-               $file_name ||= validate_input($pathname);
+               $file_name ||= $pathname;
        } elsif (defined $refname) {
                # we got "project.git/branch"
                $action ||= "shortlog";
@@ -416,7 +406,7 @@ sub validate_input {
 # correct, but quoted slashes look too horrible in bookmarks
 sub esc_param {
        my $str = shift;
-       $str =~ s/([^A-Za-z0-9\-_.~();\/;?:@&=])/sprintf("%%%02X", ord($1))/eg;
+       $str =~ s/([^A-Za-z0-9\-_.~()\/:@])/sprintf("%%%02X", ord($1))/eg;
        $str =~ s/\+/%2B/g;
        $str =~ s/ /\+/g;
        return $str;
@@ -1282,7 +1272,7 @@ sub git_header_html {
                if (defined $action) {
                        $title .= "/$action";
                        if (defined $file_name) {
-                               $title .= " - $file_name";
+                               $title .= " - " . esc_html($file_name);
                                if ($action eq "tree" && $file_name !~ m|/$|) {
                                        $title .= "/";
                                }
@@ -2430,7 +2420,7 @@ sub git_blame2 {
        if ($ftype !~ "blob") {
                die_error("400 Bad Request", "Object is not a blob");
        }
-       open ($fd, "-|", git_cmd(), "blame", '-l', $file_name, $hash_base)
+       open ($fd, "-|", git_cmd(), "blame", '-l', '--', $file_name, $hash_base)
                or die_error(undef, "Open git-blame failed");
        git_header_html();
        my $formats_nav =
@@ -3072,12 +3062,12 @@ sub git_blobdiff {
                if (defined $file_name) {
                        if (defined $file_parent) {
                                $diffinfo{'status'} = '2';
-                               $diffinfo{'from_file'} = $file_parent;
-                               $diffinfo{'to_file'}   = $file_name;
+                               $diffinfo{'from_file'} = esc_html($file_parent);
+                               $diffinfo{'to_file'}   = esc_html($file_name);
                        } else { # assume not renamed
                                $diffinfo{'status'} = '1';
-                               $diffinfo{'from_file'} = $file_name;
-                               $diffinfo{'to_file'}   = $file_name;
+                               $diffinfo{'from_file'} = esc_html($file_name);
+                               $diffinfo{'to_file'}   = esc_html($file_name);
                        }
                } else { # no filename given
                        $diffinfo{'status'} = '2';
@@ -3126,7 +3116,7 @@ sub git_blobdiff {
                        -type => 'text/plain',
                        -charset => 'utf-8',
                        -expires => $expires,
-                       -content_disposition => qq(inline; filename="${file_name}.patch"));
+                       -content_disposition => qq(inline; filename=") . quotemeta($file_name) . qq(.patch"));
 
                print "X-Git-Url: " . $cgi->self_url() . "\n\n";
 
@@ -3576,7 +3566,7 @@ sub git_rss {
                        if (!($line =~ m/^:([0-7]{6}) ([0-7]{6}) ([0-9a-fA-F]{40}) ([0-9a-fA-F]{40}) (.)([0-9]{0,3})\t(.*)$/)) {
                                next;
                        }
-                       my $file = validate_input(unquote($7));
+                       my $file = esc_html(unquote($7));
                        $file = decode("utf8", $file, Encode::FB_DEFAULT);
                        print "$file<br/>\n";
                }