Skip to content

Commit 6c630f2

Browse files
committed
Merge branch 'jk/gitweb-anti-xss'
Some codepaths in "gitweb" that forgot to escape URLs generated based on end-user input have been corrected. * jk/gitweb-anti-xss: gitweb: escape URLs generated by href() t/gitweb-lib.sh: set $REQUEST_URI t/gitweb-lib.sh: drop confusing quotes t9502: pass along all arguments in xss helper
2 parents 3288d99 + a376e37 commit 6c630f2

File tree

3 files changed

+25
-20
lines changed

3 files changed

+25
-20
lines changed

gitweb/gitweb.perl

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4048,7 +4048,7 @@ sub print_feed_meta {
40484048

40494049
$href_params{'extra_options'} = undef;
40504050
$href_params{'action'} = $type;
4051-
$link_attr{'-href'} = href(%href_params);
4051+
$link_attr{'-href'} = esc_attr(href(%href_params));
40524052
print "<link ".
40534053
"rel=\"$link_attr{'-rel'}\" ".
40544054
"title=\"$link_attr{'-title'}\" ".
@@ -4057,7 +4057,7 @@ sub print_feed_meta {
40574057
"/>\n";
40584058

40594059
$href_params{'extra_options'} = '--no-merges';
4060-
$link_attr{'-href'} = href(%href_params);
4060+
$link_attr{'-href'} = esc_attr(href(%href_params));
40614061
$link_attr{'-title'} .= ' (no merges)';
40624062
print "<link ".
40634063
"rel=\"$link_attr{'-rel'}\" ".
@@ -4070,10 +4070,12 @@ sub print_feed_meta {
40704070
} else {
40714071
printf('<link rel="alternate" title="%s projects list" '.
40724072
'href="%s" type="text/plain; charset=utf-8" />'."\n",
4073-
esc_attr($site_name), href(project=>undef, action=>"project_index"));
4073+
esc_attr($site_name),
4074+
esc_attr(href(project=>undef, action=>"project_index")));
40744075
printf('<link rel="alternate" title="%s projects feeds" '.
40754076
'href="%s" type="text/x-opml" />'."\n",
4076-
esc_attr($site_name), href(project=>undef, action=>"opml"));
4077+
esc_attr($site_name),
4078+
esc_attr(href(project=>undef, action=>"opml")));
40774079
}
40784080
}
40794081

@@ -4287,8 +4289,8 @@ sub git_footer_html {
42874289
if (defined $action &&
42884290
$action eq 'blame_incremental') {
42894291
print qq!<script type="text/javascript">\n!.
4290-
qq!startBlame("!. href(action=>"blame_data", -replay=>1) .qq!",\n!.
4291-
qq! "!. href() .qq!");\n!.
4292+
qq!startBlame("!. esc_attr(href(action=>"blame_data", -replay=>1)) .qq!",\n!.
4293+
qq! "!. esc_attr(href()) .qq!");\n!.
42924294
qq!</script>\n!;
42934295
} else {
42944296
my ($jstimezone, $tz_cookie, $datetime_class) =
@@ -7155,8 +7157,8 @@ sub git_blob {
71557157
print qq! alt="!.esc_attr($file_name).qq!" title="!.esc_attr($file_name).qq!"!;
71567158
}
71577159
print qq! src="! .
7158-
href(action=>"blob_plain", hash=>$hash,
7159-
hash_base=>$hash_base, file_name=>$file_name) .
7160+
esc_attr(href(action=>"blob_plain", hash=>$hash,
7161+
hash_base=>$hash_base, file_name=>$file_name)) .
71607162
qq!" />\n!;
71617163
} else {
71627164
my $nr;
@@ -8239,6 +8241,7 @@ sub git_feed {
82398241
} else {
82408242
$alt_url = href(-full=>1, action=>"summary");
82418243
}
8244+
$alt_url = esc_attr($alt_url);
82428245
print qq!<?xml version="1.0" encoding="utf-8"?>\n!;
82438246
if ($format eq 'rss') {
82448247
print <<XML;
@@ -8276,7 +8279,7 @@ sub git_feed {
82768279
$alt_url . '" />' . "\n" .
82778280
'<link rel="self" type="' . $content_type . '" href="' .
82788281
$cgi->self_url() . '" />' . "\n" .
8279-
"<id>" . href(-full=>1) . "</id>\n" .
8282+
"<id>" . esc_url(href(-full=>1)) . "</id>\n" .
82808283
# use project owner for feed author
82818284
"<author><name>$owner</name></author>\n";
82828285
if (defined $favicon) {
@@ -8322,7 +8325,7 @@ sub git_feed {
83228325
"<author>" . esc_html($co{'author'}) . "</author>\n" .
83238326
"<pubDate>$cd{'rfc2822'}</pubDate>\n" .
83248327
"<guid isPermaLink=\"true\">$co_url</guid>\n" .
8325-
"<link>$co_url</link>\n" .
8328+
"<link>" . esc_html($co_url) . "</link>\n" .
83268329
"<description>" . esc_html($co{'title'}) . "</description>\n" .
83278330
"<content:encoded>" .
83288331
"<![CDATA[\n";
@@ -8344,8 +8347,8 @@ sub git_feed {
83448347
}
83458348
print "</contributor>\n" .
83468349
"<published>$cd{'iso-8601'}</published>\n" .
8347-
"<link rel=\"alternate\" type=\"text/html\" href=\"$co_url\" />\n" .
8348-
"<id>$co_url</id>\n" .
8350+
"<link rel=\"alternate\" type=\"text/html\" href=\"" . esc_attr($co_url) . "\" />\n" .
8351+
"<id>" . esc_html($co_url) . "</id>\n" .
83498352
"<content type=\"xhtml\" xml:base=\"" . esc_url($my_url) . "\">\n" .
83508353
"<div xmlns=\"http://www.w3.org/1999/xhtml\">\n";
83518354
}
@@ -8452,8 +8455,8 @@ sub git_opml {
84528455
}
84538456

84548457
my $path = esc_html(chop_str($proj{'path'}, 25, 5));
8455-
my $rss = href('project' => $proj{'path'}, 'action' => 'rss', -full => 1);
8456-
my $html = href('project' => $proj{'path'}, 'action' => 'summary', -full => 1);
8458+
my $rss = esc_attr(href('project' => $proj{'path'}, 'action' => 'rss', -full => 1));
8459+
my $html = esc_attr(href('project' => $proj{'path'}, 'action' => 'summary', -full => 1));
84578460
print "<outline type=\"rss\" text=\"$path\" title=\"$path\" xmlUrl=\"$rss\" htmlUrl=\"$html\"/>\n";
84588461
}
84598462
print <<XML;

t/gitweb-lib.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,11 @@ gitweb_run () {
5858
GATEWAY_INTERFACE='CGI/1.1'
5959
HTTP_ACCEPT='*/*'
6060
REQUEST_METHOD='GET'
61-
QUERY_STRING=""$1""
62-
PATH_INFO=""$2""
61+
QUERY_STRING=$1
62+
PATH_INFO=$2
63+
REQUEST_URI=/gitweb.cgi$PATH_INFO
6364
export GATEWAY_INTERFACE HTTP_ACCEPT REQUEST_METHOD \
64-
QUERY_STRING PATH_INFO
65+
QUERY_STRING PATH_INFO REQUEST_URI
6566

6667
GITWEB_CONFIG=$(pwd)/gitweb_config.perl
6768
export GITWEB_CONFIG

t/t9502-gitweb-standalone-parse-output.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -188,8 +188,8 @@ test_expect_success 'forks: project_index lists all projects (incl. forks)' '
188188
'
189189

190190
xss() {
191-
echo >&2 "Checking $1..." &&
192-
gitweb_run "$1" &&
191+
echo >&2 "Checking $*..." &&
192+
gitweb_run "$@" &&
193193
if grep "$TAG" gitweb.body; then
194194
echo >&2 "xss: $TAG should have been quoted in output"
195195
return 1
@@ -200,7 +200,8 @@ xss() {
200200
test_expect_success 'xss checks' '
201201
TAG="<magic-xss-tag>" &&
202202
xss "a=rss&p=$TAG" &&
203-
xss "a=rss&p=foo.git&f=$TAG"
203+
xss "a=rss&p=foo.git&f=$TAG" &&
204+
xss "" "$TAG+"
204205
'
205206

206207
test_done

0 commit comments

Comments
 (0)