@matheusbsbs / matheus-bagliano / commits / 3017ed62f4
Fork 0

gitweb: Introduce esc_attr to escape attributes of HTML elements

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>

Jakub Narebski committed Dec 15, 2010 at 00:34 UTC 3017ed62f47ce14a959e2d315c434d4980cf4243
1 file changed +22 -15
gitweb/gitweb.perl
+22 -15
@@ -1084,6 +1084,13 @@ sub esc_url {
1084 return $str;
1085 }
1086
1087 +# quote unsafe characters in HTML attributes
1088 +sub esc_attr {
1089 +
1090 + # for XHTML conformance escaping '"' to '&quot;' is not enough
1091 + return esc_html(@_);
1092 +}
1093 +
1094 # replace invalid utf8 character with SUBSTITUTION sequence
1095 sub esc_html {
1096 my $str = shift;
@@ -1489,7 +1496,7 @@ sub format_ref_marker {
1496 hash=>$dest
1497 )}, $name);
1498
1492 - $markers .= " <span class=\"$class\" title=\"$ref\">" .
1499 + $markers .= " <span class=\"".esc_attr($class)."\" title=\"".esc_attr($ref)."\">" .
1500 $link . "</span>";
1501 }
1502 }
@@ -1573,7 +1580,7 @@ sub git_get_avatar {
1580 return $pre_white .
1581 "<img width=\"$size\" " .
1582 "class=\"avatar\" " .
1576 - "src=\"$url\" " .
1583 + "src=\"".esc_url($url)."\" " .
1584 "alt=\"\" " .
1585 "/>" . $post_white;
1586 } else {
@@ -2245,7 +2252,7 @@ sub git_show_project_tagcloud {
2252 } else {
2253 my @tags = sort { $cloud->{$a}->{count} <=> $cloud->{$b}->{count} } keys %$cloud;
2254 return '<p align="center">' . join (', ', map {
2248 - "<a href=\"$home_link?by_tag=$_\">$cloud->{$_}->{topname}</a>"
2255 + $cgi->a({-href=>"$home_link?by_tag=$_"}, $cloud->{$_}->{topname})
2256 } splice(@tags, 0, $count)) . '</p>';
2257 }
2258 }
@@ -3061,11 +3068,11 @@ EOF
3068 # print out each stylesheet that exist, providing backwards capability
3069 # for those people who defined $stylesheet in a config file
3070 if (defined $stylesheet) {
3064 - print '<link rel="stylesheet" type="text/css" href="'.$stylesheet.'"/>'."\n";
3071 + print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
3072 } else {
3073 foreach my $stylesheet (@stylesheets) {
3074 next unless $stylesheet;
3068 - print '<link rel="stylesheet" type="text/css" href="'.$stylesheet.'"/>'."\n";
3075 + print '<link rel="stylesheet" type="text/css" href="'.esc_url($stylesheet).'"/>'."\n";
3076 }
3077 }
3078 if (defined $project) {
@@ -3078,7 +3085,7 @@ EOF
3085 my $type = lc($format);
3086 my %link_attr = (
3087 '-rel' => 'alternate',
3081 - '-title' => "$project - $href_params{'-title'} - $format feed",
3088 + '-title' => esc_attr("$project - $href_params{'-title'} - $format feed"),
3089 '-type' => "application/$type+xml"
3090 );
3091
@@ -3105,13 +3112,13 @@ EOF
3112 } else {
3113 printf('<link rel="alternate" title="%s projects list" '.
3114 'href="%s" type="text/plain; charset=utf-8" />'."\n",
3108 - $site_name, href(project=>undef, action=>"project_index"));
3115 + esc_attr($site_name), href(project=>undef, action=>"project_index"));
3116 printf('<link rel="alternate" title="%s projects feeds" '.
3117 'href="%s" type="text/x-opml" />'."\n",
3111 - $site_name, href(project=>undef, action=>"opml"));
3118 + esc_attr($site_name), href(project=>undef, action=>"opml"));
3119 }
3120 if (defined $favicon) {
3114 - print qq(<link rel="shortcut icon" href="$favicon" type="image/png" />\n);
3121 + print qq(<link rel="shortcut icon" href=").esc_url($favicon).qq(" type="image/png" />\n);
3122 }
3123
3124 print "</head>\n" .
@@ -3124,7 +3131,7 @@ EOF
3131 print "<div class=\"page_header\">\n" .
3132 $cgi->a({-href => esc_url($logo_url),
3133 -title => $logo_label},
3127 - qq(<img src="$logo" width="72" height="27" alt="git" class="logo"/>));
3134 + qq(<img src=").esc_url($logo).qq(" width="72" height="27" alt="git" class="logo"/>));
3135 print $cgi->a({-href => esc_url($home_link)}, $home_link_str) . " / ";
3136 if (defined $project) {
3137 print $cgi->a({-href => href(action=>"summary")}, esc_html($project));
@@ -5016,14 +5023,14 @@ sub git_blob {
5023 } else {
5024 print "<div class=\"page_nav\">\n" .
5025 "<br/><br/></div>\n" .
5019 - "<div class=\"title\">$hash</div>\n";
5026 + "<div class=\"title\">".esc_html($hash)."</div>\n";
5027 }
5028 git_print_page_path($file_name, "blob", $hash_base);
5029 print "<div class=\"page_body\">\n";
5030 if ($mimetype =~ m!^image/!) {
5024 - print qq!<img type="$mimetype"!;
5031 + print qq!<img type="!.esc_attr($mimetype).qq!"!;
5032 if ($file_name) {
5026 - print qq! alt="$file_name" title="$file_name"!;
5033 + print qq! alt="!.esc_attr($file_name).qq!" title="!.esc_attr($file_name).qq!"!;
5034 }
5035 print qq! src="! .
5036 href(action=>"blob_plain", hash=>$hash,
@@ -5094,7 +5101,7 @@ sub git_tree {
5101 undef $hash_base;
5102 print "<div class=\"page_nav\">\n";
5103 print "<br/><br/></div>\n";
5097 - print "<div class=\"title\">$hash</div>\n";
5104 + print "<div class=\"title\">".esc_html($hash)."</div>\n";
5105 }
5106 if (defined $file_name) {
5107 $basedir = $file_name;
@@ -5511,7 +5518,7 @@ sub git_blobdiff {
5518 git_print_header_div('commit', esc_html($co{'title'}), $hash_base);
5519 } else {
5520 print "<div class=\"page_nav\"><br/>$formats_nav<br/></div>\n";
5514 - print "<div class=\"title\">$hash vs $hash_parent</div>\n";
5521 + print "<div class=\"title\">".esc_html("$hash vs $hash_parent")."</div>\n";
5522 }
5523 if (defined $file_name) {
5524 git_print_page_path($file_name, "blob", $hash_base);