{"thread":{"id":"17641","subject":"[PATCH] gitweb: add $prevent_xss option to prevent XSS by repository content","startedAt":"2009-02-08T00:00:09Z","lastAt":"2009-02-08T00:00:09Z","messageCount":1,"participants":["Matt McCutchen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103672","messageId":"1234051209.3299.26.camel@mattlaptop2.local","threadId":"17641","inReplyTo":null,"subject":"[PATCH] gitweb: add $prevent_xss option to prevent XSS by repository content","fromName":"Matt McCutchen","fromEmail":"matt@mattmccutchen.net","sentAt":"2009-02-08T00:00:09Z","receivedAt":"2009-02-08T00:00:09Z","isPatch":true,"sender":{"key":"matt@mattmccutchen.net","avatar":"https://avatars.githubusercontent.com/u/8885753?v=4"},"body":"Add a gitweb configuration variable $prevent_xss that disables features\nto prevent content in repositories from launching cross-site scripting\n(XSS) attacks in the gitweb domain.  Currently, this option makes gitweb\nignore README.html (a better solution may be worked out in the future)\nand serve a blob_plain file of an untrusted type with\n\"Content-Disposition: attachment\", which tells the browser not to show\nthe file at its original URL.\n\nThe XSS prevention is currently off by default.\n\nSigned-off-by: Matt McCutchen <matt@mattmccutchen.net>\n---\n\nI found these vulnerabilities a few weeks ago during a security audit of\nmy Web site and discussed them with Junio Hamano, Jeff King, and Jakub\nNarebski.  Unfortunately, it's hard to close them without a loss of\nfunctionality that may be unacceptable to some sites.  We decided to go\nahead and publicize the issue with this patch, which will give sites a\nchoice to keep the functionality or prevent XSS depending on their\nneeds.  Input is invited from webmasters on a better solution to be\nadded in the future.\n\nMatt\n\n gitweb/README      |    9 ++++++++-\n gitweb/gitweb.perl |   21 +++++++++++++++++++--\n 2 files changed, 27 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/README b/gitweb/README\nindex a9dc2e5..8433dd1 100644\n--- a/gitweb/README\n+++ b/gitweb/README\n@@ -212,6 +212,11 @@ not include variables usually directly set during build):\n    Rename detection options for git-diff and git-diff-tree. By default\n    ('-M'); set it to ('-C') or ('-C', '-C') to also detect copies, or\n    set it to () if you don't want to have renames detection.\n+ * $prevent_xss\n+   If true, some gitweb features are disabled to prevent content in\n+   repositories from launching cross-site scripting (XSS) attacks.  Set this\n+   to true if you don't trust the content of your repositories. The default\n+   is false.\n \n\n Projects list file format\n@@ -258,7 +263,9 @@ You can use the following files in repository:\n    A .html file (HTML fragment) which is included on the gitweb project\n    summary page inside <div> block element. You can use it for longer\n    description of a project, to provide links (for example to project's\n-   homepage), etc.\n+   homepage), etc. This is recognized only if XSS prevention is off\n+   ($prevent_xss is false); a way to include a readme safely when XSS\n+   prevention is on may be worked out in the future.\n  * description (or gitweb.description)\n    Short (shortened by default to 25 characters in the projects list page)\n    single line description of a project (of a repository). Plain text file;\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex f27dbb6..5410874 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -132,6 +132,10 @@ our $fallback_encoding = 'latin1';\n # - one might want to include '-B' option, e.g. '-B', '-M'\n our @diff_opts = ('-M'); # taken from git_commit\n \n+# Disables features that would allow repository owners to inject script into\n+# the gitweb domain.\n+our $prevent_xss = 0;\n+\n # information about snapshot formats that gitweb is capable of serving\n our %known_snapshot_formats = (\n \t# name => {\n@@ -4503,7 +4507,9 @@ sub git_summary {\n \n \tprint \"</table>\\n\";\n \n-\tif (-s \"$projectroot/$project/README.html\") {\n+\t# If XSS prevention is on, we don't include README.html.\n+\t# TODO: Allow a readme in some safe format.\n+\tif (!$prevent_xss && -s \"$projectroot/$project/README.html\") {\n \t\tprint \"<div class=\\\"title\\\">readme</div>\\n\" .\n \t\t      \"<div class=\\\"readme\\\">\\n\";\n \t\tinsert_file(\"$projectroot/$project/README.html\");\n@@ -4764,10 +4770,21 @@ sub git_blob_plain {\n \t\t$save_as .= '.txt';\n \t}\n \n+\t# With XSS prevention on, blobs of all types except a few known safe\n+\t# ones are served with \"Content-Disposition: attachment\" to make sure\n+\t# they don't run in our security domain.  For certain image types,\n+\t# blob view writes an <img> tag referring to blob_plain view, and we\n+\t# want to be sure not to break that by serving the image as an\n+\t# attachment (though Firefox 3 doesn't seem to care).\n+\tmy $sandbox = $prevent_xss &&\n+\t\t$type !~ m!^(?:text/plain|image/(?:gif|png|jpeg))$!;\n+\n \tprint $cgi->header(\n \t\t-type => $type,\n \t\t-expires => $expires,\n-\t\t-content_disposition => 'inline; filename=\"' . $save_as . '\"');\n+\t\t-content_disposition =>\n+\t\t\t($sandbox ? 'attachment' : 'inline')\n+\t\t\t. '; filename=\"' . $save_as . '\"');\n \tundef $/;\n \tbinmode STDOUT, ':raw';\n \tprint <$fd>;\n-- \n1.6.2.rc0.7.g013dd.dirty\n"}]}