threads / discuss / 27734

[PATCHv2 0/2] gitweb: Make $prevent_xss protection for 'blob_plain' more usable

Subject: [PATCHv2 0/2] gitweb: Make $prevent_xss protection for 'blob_plain' more usable

## tl;dr

4 messages between Jun 30, 2011 and Jun 30, 2011.

replies: 3people: 2as markdown or json

Jakub Narebski· Jun 30, 2011, 09:39 UTC · lore

This series is improvement and extending of patch with the same name, containing only the first patch in the series, sent to git mailing list on 10.06.2011, and present in 'pu' as fb76adb (jn/mime-type-with-params).

This series is to replace it.

The original impulse behind creating this series was the fact that the summary of previously sent commit was quite cryptic, and didn't really explain what it was intended to do. While at it I have added one more simplification on top of the one proposed by Junio.

The second path in this series is to have gitweb treat the same 'blob_plain' view of both *.xhtml (with application/xhtml+xml mimetype) and *.html (text/html), when $prevent_xss is on.

Jakub Narebski (2):
  gitweb: Serve text/*  'blob_plain' as text/plain with $prevent_xss
  gitweb: Serve */*+xml 'blob_plain' as text/plain with $prevent_xss

 gitweb/gitweb.perl |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

-- 
1.7.5

Jakub Narebski· Jun 30, 2011, 09:39 UTC · re: Jakub Narebski · lore

[PATCH 1/2] gitweb: Serve text/* 'blob_plain' as text/plain with $prevent_xss

One of mechanism enabled by setting $prevent_xss to true is 'blob_plain' view protection. 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.

Instead of serving text/* type files, except text/plain (and including text/html), as attachements, downgrade it to text/plain. This way HTML pages in 'blob_plain' (raw) wiew would be displayed in browser, but safely as a source, and not asked to be saved.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
 gitweb/gitweb.perl |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index 8620aca..cb2e7bc 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6139,7 +6139,15 @@ 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/[a-z]+|image/(?:gif|png|jpeg))(?:[ ;]|$)!;
+
+	# serve text/* as text/plain
+	if ($prevent_xss &&
+	    $type =~ m!^text/[a-z]+\b(.*)$!) {
+		my $rest = $1;
+		$rest = defined $rest ? $rest : '';
+		$type = "text/plain$rest";
+	}
 
 	print $cgi->header(
 		-type => $type,

-- 
1.7.5

Junio C Hamano· Jun 30, 2011, 18:22 UTC · re: Jakub Narebski · lore

Re: [PATCH 1/2] gitweb: Serve text/* 'blob_plain' as text/plain with $prevent_xss

Jakub Narebski <jnareb@gmail.com> writes:

Show 8 quoted lines

> One of mechanism enabled by setting $prevent_xss to true is 'blob_plain'
> view protection.  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.
>
> Instead of serving text/* type files, except text/plain (and including
> text/html), as attachements, downgrade it to text/plain.  This way HTML
> pages in 'blob_plain' (raw) wiew would be displayed in browser, but

A new typo "wiew" is introduced without touching other parts of the message. Curious...

Show 28 quoted lines

> safely as a source, and not asked to be saved.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
>  gitweb/gitweb.perl |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8620aca..cb2e7bc 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -6139,7 +6139,15 @@ 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/[a-z]+|image/(?:gif|png|jpeg))(?:[ ;]|$)!;
> +
> +	# serve text/* as text/plain
> +	if ($prevent_xss &&
> +	    $type =~ m!^text/[a-z]+\b(.*)$!) {
> +		my $rest = $1;
> +		$rest = defined $rest ? $rest : '';
> +		$type = "text/plain$rest";
> +	}
>  
>  	print $cgi->header(
>  		-type => $type,

Jakub Narebski· Jun 30, 2011, 09:39 UTC · re: Jakub Narebski · lore

[PATCHv1 2/2] gitweb: Serve */*+xml 'blob_plain' as text/plain with $prevent_xss

Enhance usability of 'blob_plain' view protection against XSS attacks (enabled by setting $prevent_xss to true) by serving contents inline as safe 'text/plain' mimetype where possible, instead of serving with "Content-Disposition: attachment" to make sure they don't run in gitweb's security domain.

This patch broadens downgrading to 'text/plain' further, to any */*+xml mimetype. This includes:

  application/xhtml+xml    (*.xhtml, *.xht)
  application/atom+xml     (*.atom)
  application/rss+xml      (*.rss)
  application/mathml+xm    (*.mathml)
  application/docbook+xml  (*.docbook)
  image/svg+xml            (*.svg, *.svgz)

Probably most useful is serving XHTML files as text/plain in 'blob_plain' view, directly viewable.

Because file with 'image/svg+xml' mimetype can be compressed SVGZ file, we have to check if */*+xml really is text file, via '-T $fd'.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
This patch is new.

 gitweb/gitweb.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index cb2e7bc..ae6244c 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -6143,7 +6143,8 @@ sub git_blob_plain {
 
 	# serve text/* as text/plain
 	if ($prevent_xss &&
-	    $type =~ m!^text/[a-z]+\b(.*)$!) {
+	    ($type =~ m!^text/[a-z]+\b(.*)$! ||
+	     ($type =~ m!^[a-z]+/[a-z]\+xml\b(.*)$! && -T $fd))) {
 		my $rest = $1;
 		$rest = defined $rest ? $rest : '';
 		$type = "text/plain$rest";

-- 
1.7.5