oss-security - Re: XSS security issue in gitweb for 'blob

Openwall GNU/*/Linux 3.0 - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
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
Please check out the Open Source Software Security Wiki, which is counterpart to this mailing list.