oss-sec mailing list archives

Re: XSS security issue in gitweb for 'blob_plain' view with HTML files


From: Jakub Narebski <jnareb () gmail com>
Date: Sat, 4 Jun 2011 11:27:59 +0200

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 () gmail com>
  http://permalink.gmane.org/gmane.comp.version-control.git/175057

-- 
Jakub Narebski
Poland


Current thread: