On Mon, Nov 12, 2012 at 01:55:46PM -0500, Drew Northup wrote:
> On Sun, Nov 11, 2012 at 6:28 PM, glpk xypron <xypron.glpk@xxxxxx> wrote:
> > Gitweb can be used to generate an RSS feed.
> >
> > Arbitrary tags can be inserted into the XML document describing
> > the RSS feed by careful construction of the URL.
> [...]
> Something like this may be useful to defuse the "file" parameter, but
> I presume a more definitive fix is in order...
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 10ed9e5..af93e65 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1447,6 +1447,10 @@ sub validate_pathname {
> if ($input =~ m!\0!) {
> return undef;
> }
> + # No XSS <script></script> inclusions
> + if ($input =~ m!(<script>)(.*)(</script>)!){
> + return undef;
> + }
> return $input;
> }
This is the wrong fix for a few reasons:
1. It is on the input-validation side, whereas the real problem is on
the output-quoting side. Your patch means I could not access a file
called "<script>foo</script>". What we really want is to have the
unquoted name internally, but then make sure we quote it when
outputting as part of an HTML (or XML) file.
2. Script tags are only part of the problem. They are what make it
obviously a security vulnerability, but it is equally incorrect for
us to show the filename "<b>foo</b>" as bold. I would also not be
surprised if there are other cross-site attacks one can do without
using <script>.
3. Your filter is too simplistic. At the very least, it would not
filter out "<SCRIPT>". I am not up to date on all of the
sneaking-around-HTML-filters attacks that are available these days,
but I wonder if one could also get around it using XML entities or
similar.
I think the right answer is going to be a well-placed call to esc_html.
This already happens automatically when we go through the CGI
element-building functions, but obviously we failed to make the call
when building the output manually. This is a great reason why template
languages which default to safe expansion should always be used.
Unfortunately, gitweb is living in 1995 in terms of web frameworks.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html