From mboxrd@z Thu Jan  1 00:00:00 1970
From: john at keeping.me.uk (John Keeping)
Date: Thu, 14 Jan 2016 10:57:23 +0000
Subject: XSS in cgit
In-Reply-To: <CAHmME9pf_TjMHYiNuqC4fF5i69w+ntV+Rgv_Tt5c2Y7MnCtgKA@mail.gmail.com>
References: <CAHmME9pf_TjMHYiNuqC4fF5i69w+ntV+Rgv_Tt5c2Y7MnCtgKA@mail.gmail.com>
Message-ID: <20160114105723.GH14056@serenity.lan>

On Wed, Jan 13, 2016 at 05:07:12PM +0100, Jason A. Donenfeld wrote:
> First (1), the big bad one. In ui-blob.c, we have:
> 
> ctx.page.mimetype = ctx.qry.mimetype;
> cgit_print_http_headers();
> 
> This invokes, from ui-shared.c:
>   htmlf("Content-Type: %s\n", ctx.page.mimetype);
> or
>   htmlf("Content-Type: %s; charset=%s\n", ctx.page.mimetype, ctx.page.charset);
> 
> 
> A malicious user can pass a mime type such as text/html followed by a
> few new lines and then some malicious javascript in a script tag to
> launch an XSS attack. The obvious solution here is to ensure
> ctx.page.mimetype doesn't contain new lines, null characters, and
> other naughty fields according to the HTTP spec.

I wonder if we should just drop support for the "mimetype" query
parameter and see if anyone complains.  In general, I would expect it to
be the server's responsibility to decide on the type of its output and
allowing the client to override it seems like a problem in general.