CVE-2019-16216: Fix MIME type validation.

* Whitelist a small number of image/ types to be served as
  non-attachments.
* Serve the file using the type that we validated rather than relying
  on an independent guess to match.

This issue can lead to a stored XSS security vulnerability for older
browsers that don't support Content-Security-Policy.

It primarily affects servers using Zulip's local file uploads backend
for servers running Ubuntu 16.04 Xenial or newer; the legacy local
file upload backend for (now EOL) Ubuntu 14.04 Trusty was not affected
and it has limited impact for the S3 upload backend (which uses an
unprivileged S3 bucket domain to serve files).

This was fixed in master via 780ecb672b.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>

zerver/lib/upload.py

@@ -1,4 +1,4 @@
-from typing import Dict, Optional, Tuple
+from typing import Optional, Tuple
 
 from django.utils.translation import ugettext as _
 from django.conf import settings
@@ -41,6 +41,17 @@ DEFAULT_EMOJI_SIZE = 64
 MAX_EMOJI_GIF_SIZE = 128
 MAX_EMOJI_GIF_FILE_SIZE_BYTES = 128 * 1024 * 1024  # 128 kb
 
+INLINE_MIME_TYPES = [
+    "application/pdf",
+    "image/gif",
+    "image/jpeg",
+    "image/png",
+    "image/webp",
+    # To avoid cross-site scripting attacks, DO NOT add types such
+    # as application/xhtml+xml, application/x-shockwave-flash,
+    # image/svg+xml, text/html, or text/xml.
+]
+
 # Performance Note:
 #
 # For writing files to S3, the file could either be stored in RAM
@@ -267,10 +278,11 @@ def upload_image_to_s3(
     key.set_metadata("user_profile_id", str(user_profile.id))
     key.set_metadata("realm_id", str(user_profile.realm_id))
 
+    headers = {}
     if content_type is not None:
-        headers = {'Content-Type': content_type}  # type: Optional[Dict[str, str]]
-    else:
-        headers = None
+        headers["Content-Type"] = content_type
+    if content_type not in INLINE_MIME_TYPES:
+        headers["Content-Disposition"] = "attachment"
 
     key.set_contents_from_string(contents, headers=headers)  # type: ignore # https://github.com/python/typeshed/issues/1552
 

zerver/views/upload.py

@@ -7,7 +7,7 @@ from django.utils.translation import ugettext as _
 
 from zerver.lib.response import json_success, json_error
 from zerver.lib.upload import upload_message_image_from_request, get_local_file_path, \
-    get_signed_upload_url, check_upload_within_quota
+    get_signed_upload_url, check_upload_within_quota, INLINE_MIME_TYPES
 from zerver.models import UserProfile, validate_attachment_request
 from django.conf import settings
 from sendfile import sendfile
@@ -38,13 +38,11 @@ def serve_local(request: HttpRequest, path_id: str) -> HttpResponse:
     # consistent format (urlquoted).  For more details on filename*
     # and filename, see the below docs:
     # https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition
-    attachment = True
-    file_type = guess_type(local_path)[0]
-    if file_type is not None and (file_type.startswith("image/") or
-                                  file_type == "application/pdf"):
-        attachment = False
+    mimetype, encoding = guess_type(local_path)
+    attachment = mimetype not in INLINE_MIME_TYPES
 
-    return sendfile(request, local_path, attachment=attachment)
+    return sendfile(request, local_path, attachment=attachment,
+                    mimetype=mimetype, encoding=encoding)
 
 def serve_file_backend(request: HttpRequest, user_profile: UserProfile,
                        realm_id_str: str, filename: str) -> HttpResponse: