[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <201106041128.03896.jnareb@gmail.com>
Date: Sat, 4 Jun 2011 11:27:59 +0200
From: Jakub Narebski <jnareb@...il.com>
To: Jamie Strandboge <jamie@...onical.com>
Cc: Junio C Hamano <gitster@...ox.com>,
oss-security@...ts.openwall.com,
dave b <db.pub.mail@...il.com>
Subject: Re: XSS security issue in gitweb for 'blob_plain' view with HTML files
On Fri, 3 July 2011, Jakub Narebski wrote:
> On Fri, 3 July 2011, Jamie Strandboge wrote:
>
> > https://launchpad.net/bugs/777804
> [...]
> > ----
> > I am reporting a persistent xss vector in gitweb, note this requires a
> > user to have commit access to a repository that gitweb is configured
> > to display. The vector is the fact that gitweb "serves" up xml files -
> > which can (just as gitweb does) embed html that could be used to
> > perform a cross-site scripting attack.
> >
> > e.g. (lol.xml).
> > <?xml version="1.0" encoding="utf-8"?>
> > <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN"
> > "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
> > <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en-US" lang="en-US">
> > <head>
> > </head>
> > <script>alert(1);</script>
> > </html>
> >
> > and viewed at
> > http://$HOSTNAME/$PATH_TO_GITWEB/?p=lolok;a=blob_plain;f=lol.xml
> > ----
> >
> > Thanks in advance for your cooperation in coordinating a fix for this
> > issue,
>
> In short: This is a feature, not a bug.
>
> Origin of current behavior:
> ---------------------------
> The 'blob_plain' action (raw view) together with support for path_info
> URLs were designed together so that gitweb could be used as a kind of
> deploy platform. For example you can browse git documentation from
> 'html' branch of git.git repository using gitweb, c.f.
>
> http://repo.or.cz/w/git.git/blob_plain/html:/git.html
>
> Also, by default (and I think in most configurations) there isn't
> anything worth stealing using cross-side scripting attack; there
> is no login information, no cookies with sensitive information...
>
> Proposal of solution:
> ---------------------
> Nevertheless gitweb include a germ of anti-XSS framework, namely
> $prevent_xss gitweb configuration variable.
It was introduced by Matt McCutchen in commit 7e1100e (gitweb: add
$prevent_xss option to prevent XSS by repository content, 2009-02-07),
and version 1.6.1.4 / 1.6.2.
> It is currently used to prevent displaying README.html from $GIT_DIR
> of repository, but I think it can be reused for this situation (at
> the cost of reduced feature set). Namely if $prevent_xss is true,
> we can simply serve all 'blob_plain' as either text/plain or
> application/octet-stream (with possible exception of *.jpg, *.gif
> and *.png images).
Actually what I haven't noticed $prevent_xss does more than that:
# With XSS prevention on, blobs of all types except a few known safe
# ones are served with "Content-Disposition: attachment" to make sure
# they don't run in our security domain. For certain image types,
# blob view writes an <img> tag referring to blob_plain view, and we
# want to be sure not to break that by serving the image as an
# attachment (though Firefox 3 doesn't seem to care).
my $sandbox = $prevent_xss &&
$type !~ m!^(?:text/plain|image/(?:gif|png|jpeg))(?:[ ;]|$)!;
> Proposed patch:
> ---------------
> Note that it includes unrelated fix for $prevent_xss feature. It would
> be split in separate patch (non-security related bugfix).
>
> With this patch above lol.xml would be served as text/plain...
>
> -- >8 --
> diff --git i/gitweb/gitweb.perl w/gitweb/gitweb.perl
> index 240dd47..a3c03f3 100755
> --- i/gitweb/gitweb.perl
> +++ w/gitweb/gitweb.perl
> @@ -3595,7 +3595,7 @@ sub blob_mimetype {
> my $fd = shift;
> my $filename = shift;
>
> - if ($filename) {
> + if ($filename && !$prevent_xss) {
> my $mime = mimetype_guess($filename);
> $mime and return $mime;
> }
So I think the above is not necessary; it is enough to enable XSS
prevention by adding
our $prevent_xss = 1;
in gitweb configuration file.
> @@ -6127,7 +6127,7 @@ sub git_blob_plain {
> # want to be sure not to break that by serving the image as an
> # attachment (though Firefox 3 doesn't seem to care).
> my $sandbox = $prevent_xss &&
> - $type !~ m!^(?:text/plain|image/(?:gif|png|jpeg))$!;
> + $type !~ m!^(?:text/plain(?:; ?charset=.*)|image/(?:gif|png|jpeg))$!;
>
> print $cgi->header(
> -type => $type,
> -- 8< --
This unrelated fix was sent in slightly different form to git mailing
list (as being non security related) as
Subject: [PATCH] gitweb: Fix usability of $prevent_xss
Message-ID: <1307177015-880-1-git-send-email-jnareb@...il.com>
http://permalink.gmane.org/gmane.comp.version-control.git/175057
--
Jakub Narebski
Poland
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
