Skip to content

Commit

Permalink
gitweb: Introduce esc_attr to escape attributes of HTML elements
Browse files Browse the repository at this point in the history
It is needed only to escape attributes of handcrafted HTML elements,
and not those generated using CGI.pm subroutines / methods for HTML
generation.

While at it, add esc_url and esc_html where needed, and prefer to use
CGI.pm HTML generating methods than handcrafted HTML code.  Most of
those are probably unnecessary (could be exploited only by person with
write access to gitweb config, or at least access to the repository).

This fixes CVE-2010-3906

Reported-by: Emanuele Gentili <e.gentili@tigersecurity.it>
Helped-by: John 'Warthog9' Hawley <warthog9@kernel.org>
Helped-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jakub Narebski authored and Junio C Hamano committed Dec 15, 2010
1 parent 1b0b962 commit 3017ed6
Showing 1 changed file with 22 additions and 15 deletions.
37 changes: 22 additions & 15 deletions gitweb/gitweb.perl
Original file line number Diff line number Diff line change
Expand Up @@ -1084,6 +1084,13 @@ sub esc_url {
return $str;
}

# quote unsafe characters in HTML attributes
sub esc_attr {

# for XHTML conformance escaping '"' to '&quot;' is not enough
return esc_html(@_);
}

# replace invalid utf8 character with SUBSTITUTION sequence
sub esc_html {
my $str = shift;
Expand Down Expand Up @@ -1489,7 +1496,7 @@ sub format_ref_marker {
hash=>$dest
)}, $name);

$markers .= " <span class=\"$class\" title=\"$ref\">" .
$markers .= " <span class=\"".esc_attr($class)."\" title=\"".esc_attr($ref)."\">" .
$link . "</span>";
}
}
Expand Down Expand Up @@ -1573,7 +1580,7 @@ sub git_get_avatar {
return $pre_white .
"<img width=\"$size\" " .
"class=\"avatar\" " .
"src=\"$url\" " .
"src=\"".esc_url($url)."\" " .
"alt=\"\" " .
"/>" . $post_white;
} else {
Expand Down Expand Up @@ -2245,7 +2252,7 @@ sub git_show_project_tagcloud {
} else {
my @tags = sort { $cloud->{$a}->{count} <=> $cloud->{$b}->{count} } keys %$cloud;
return '<p align="center">' . join (', ', map {
"<a href=\"$home_link?by_tag=$_\">$cloud->{$_}->{topname}</a>"
$cgi->a({-href=>"$home_link?by_tag=$_"}, $cloud->{$_}->{topname})
} splice(@tags, 0, $count)) . '</p>';
}
}
Expand Down Expand Up @@ -3061,11 +3068,11 @@ sub git_header_html {
# print out each stylesheet that exist, providing backwards capability
# for those people who defined $stylesheet in a config file
if (defined $stylesheet) {
print '<link rel="stylesheet" type="text/css" href="'.$stylesheet.'"/>'."\n";
print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
} else {
foreach my $stylesheet (@stylesheets) {
next unless $stylesheet;
print '<link rel="stylesheet" type="text/css" href="'.$stylesheet.'"/>'."\n";
print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
}
}
if (defined $project) {
Expand All @@ -3078,7 +3085,7 @@ sub git_header_html {
my $type = lc($format);
my %link_attr = (
'-rel' => 'alternate',
'-title' => "$project - $href_params{'-title'} - $format feed",
'-title' => esc_attr("$project - $href_params{'-title'} - $format feed"),
'-type' => "application/$type+xml"
);

Expand All @@ -3105,13 +3112,13 @@ sub git_header_html {
} else {
printf('<link rel="alternate" title="%s projects list" '.
'href="%s" type="text/plain; charset=utf-8" />'."\n",
$site_name, href(project=>undef, action=>"project_index"));
esc_attr($site_name), href(project=>undef, action=>"project_index"));
printf('<link rel="alternate" title="%s projects feeds" '.
'href="%s" type="text/x-opml" />'."\n",
$site_name, href(project=>undef, action=>"opml"));
esc_attr($site_name), href(project=>undef, action=>"opml"));
}
if (defined $favicon) {
print qq(<link rel="shortcut icon" href="$favicon" type="image/png" />\n);
print qq(<link rel="shortcut icon" href=").esc_url($favicon).qq(" type="image/png" />\n);
}

print "</head>\n" .
Expand All @@ -3124,7 +3131,7 @@ sub git_header_html {
print "<div class=\"page_header\">\n" .
$cgi->a({-href => esc_url($logo_url),
-title => $logo_label},
qq(<img src="$logo" width="72" height="27" alt="git" class="logo"/>));
qq(<img src=").esc_url($logo).qq(" width="72" height="27" alt="git" class="logo"/>));
print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
if (defined $project) {
print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
Expand Down Expand Up @@ -5016,14 +5023,14 @@ sub git_blob {
} else {
print "<div class=\"page_nav\">\n" .
"<br/><br/></div>\n" .
"<div class=\"title\">$hash</div>\n";
"<div class=\"title\">".esc_html($hash)."</div>\n";
}
git_print_page_path($file_name, "blob", $hash_base);
print "<div class=\"page_body\">\n";
if ($mimetype =~ m!^image/!) {
print qq!<img type="$mimetype"!;
print qq!<img type="!.esc_attr($mimetype).qq!"!;
if ($file_name) {
print qq! alt="$file_name" title="$file_name"!;
print qq! alt="!.esc_attr($file_name).qq!" title="!.esc_attr($file_name).qq!"!;
}
print qq! src="! .
href(action=>"blob_plain", hash=>$hash,
Expand Down Expand Up @@ -5094,7 +5101,7 @@ sub git_tree {
undef $hash_base;
print "<div class=\"page_nav\">\n";
print "<br/><br/></div>\n";
print "<div class=\"title\">$hash</div>\n";
print "<div class=\"title\">".esc_html($hash)."</div>\n";
}
if (defined $file_name) {
$basedir = $file_name;
Expand Down Expand Up @@ -5511,7 +5518,7 @@ sub git_blobdiff {
git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
} else {
print "<div class=\"page_nav\"><br/>$formats_nav<br/></div>\n";
print "<div class=\"title\">$hash vs $hash_parent</div>\n";
print "<div class=\"title\">".esc_html("$hash vs $hash_parent")."</div>\n";
}
if (defined $file_name) {
git_print_page_path($file_name, "blob", $hash_base);
Expand Down

0 comments on commit 3017ed6

Please sign in to comment.