List for cgit developers and users
 help / color / mirror / Atom feed
From: Jason at zx2c4.com (Jason A. Donenfeld)
Subject: XSS in cgit
Date: Wed, 13 Jan 2016 17:07:12 +0100	[thread overview]
Message-ID: <CAHmME9pf_TjMHYiNuqC4fF5i69w+ntV+Rgv_Tt5c2Y7MnCtgKA@mail.gmail.com> (raw)

Hi folks,

Krzysztof Katowicz-Kowalewsk has contacted me about two XSS security
vulnerabilities he's found in cgit. Upon investigation the problem
looks a bit deeper than he initially reported, and I found a few more
related vulnerabilities, and I think it generally deserves a bit of
discussion for the best way of patching it.

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.

Second (2), there's another place where something similar happens:
  htmlf("Content-Disposition: inline; filename=\"%s\"\n", ctx.page.filename);

In this case, ctx.page.filename is related to what the user has put in
the git archive. Filenames could contain new lines, resulting in a
similar vulnerability.

Thus, for (1) and (2), I will gladly accept patches that run
parameters to header functions through a sanitation function.

Third (3), a user may upload an HTML page to a git repository. Upon
fetching it via /blob, it will use the inferred mime-type of
text/html. Though I have in the past enjoyed this as a means of
serving HTML from my personal cgit, this is probably not a desired
state of affairs for folks hosting cgit in shared environments, or
using cgit to mirror third party repositories. I therefore believe we
should use either a whitelist or a blacklist for allowed mime-types.
Furthermore, before dumping these blobs, we should add these two
headers:

        html("X-Content-Type-Options: nosniff\n");
        html("Content-Security-Policy: default-src 'none'\n");

I will thus accept patches for (3) that somehow safely limit the
mime-types served for /blob and /plain. If a whitelist,
ctx.qry.mimetype can then be removed entirely. If a blacklist, we can
keep ctx.qry.mimetype, but we'll need to be ensure that there's a
future proof way of preventing XSS via a mimetype blacklist; I'm not
yet convinced such a list can be compiled securely.

Given all this, could somebody remind me why we have both /plain and
/blob handlers? And if it's still necessary to maintain a distinction?
If not, I will gladly accept patches to unify these.

Thanks,
Jason


             reply	other threads:[~2016-01-13 16:07 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-13 16:07 Jason [this message]
2016-01-13 19:11 ` normalperson
2016-01-15 11:34   ` Jason
2016-01-15 15:18     ` Jason
     [not found]       ` <7B8B10EF-8DCA-4115-9D33-4DD56F670BAB@klever.net>
2016-01-16  0:23         ` Jason
2016-01-16  9:38           ` john
2016-01-17 16:23         ` Fwd: " Jason
2016-01-14 10:57 ` john
2016-01-14 11:01   ` Jason
2016-01-14 11:07     ` john
2016-01-14 11:53       ` mailings
2016-01-14 12:59         ` Jason
2016-01-14 13:35           ` Jason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHmME9pf_TjMHYiNuqC4fF5i69w+ntV+Rgv_Tt5c2Y7MnCtgKA@mail.gmail.com \
    --to=cgit@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).