Re: [BUG] gitweb: XSS vulnerability of RSS feed
- From
Jeff King <peff@peff.net>
- Date
- Nov 12, 2012, 21:34 UTC
- Message-ID
- <20121112213428.GK4623@sigill.intra.peff.net>
- In-Reply-To
- <CANQwDwdRTeaVS5cMic5gv9SP1A8Z1vruOsZBFfMDQDTZHBAtvQ@mail.gmail.com>
On Mon, Nov 12, 2012 at 10:13:27PM +0100, Jakub Narębski wrote:
Show 7 quoted lines
> > Yeah, that looks correct, given the way how the other variables > > emitted with the same "print" like $descr and $owner are formed. > > It looks like good solution to me too. > > Nb. the problems with feed are mainly because it is generated > by hand even more than HTML (which uses CGI.pm).
Yeah, I noticed that. Here it is in patch form with a test. It would be nice if people interested in gitweb would add more entries to the XSS test below (I put in the one that fails, along with an obvious variation that is actually OK).
I didn't look carefully through the rest of gitweb for more XSS instances. From a glance, it looks like we mostly use the safe CGI methods, but probably it could use a full audit (which again, I would be happy if people who care more about gitweb would do).
-- >8 -- Subject: [PATCH] gitweb: escape html in rss title
The title of an RSS feed is generated from many components, including the filename provided as a query parameter, but we failed to quote it. Besides showing the wrong output, this is a vector for XSS attacks.
Signed-off-by: Jeff King <peff@peff.net> --- gitweb/gitweb.perl | 1 + t/t9502-gitweb-standalone-parse-output.sh | 15 +++++++++++++++ 2 files changed, 16 insertions(+)
diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl index 10ed9e5..a51a8ba 100755 --- a/gitweb/gitweb.perl +++ b/gitweb/gitweb.perl @@ -8055,6 +8055,7 @@ sub git_feed { $feed_type = 'history'; } $title .= " $feed_type"; + $title = esc_html($title); my $descr = git_get_project_description($project); if (defined $descr) { $descr = esc_html($descr); diff --git a/t/t9502-gitweb-standalone-parse-output.sh b/t/t9502-gitweb-standalone-parse-output.sh index 731e64c..3a8e7d3 100755 --- a/t/t9502-gitweb-standalone-parse-output.sh +++ b/t/t9502-gitweb-standalone-parse-output.sh @@ -185,5 +185,20 @@ test_expect_success 'forks: project_index lists all projects (incl. forks)' ' test_cmp expected actual ' +xss() { + echo >&2 "Checking $1..." && + gitweb_run "$1" && + if grep "$TAG" gitweb.body; then + echo >&2 "xss: $TAG should have been quoted in output" + return 1 + fi + return 0 +} + +test_expect_success 'xss checks' ' + TAG="<magic-xss-tag>" && + xss "a=rss&p=$TAG" && + xss "a=rss&p=foo.git&f=$TAG" +' test_done
-- 1.8.0.207.gdf2154c