CVE-2023-22735 (Python, published 2023-02-07T19:15Z)
Description: value: Zulip is an open-source team collaboration tool. In versions of zulip prior to commit `2f6c5a8` but after commit `04cf68b` users could upload files with arbitrary `Content-Type` which would be served from the Zulip hostname with `Content-Disposition: inline` and no `Content-Security-Policy` header, allowing them to trick other users into executing arbitrary Javascript in the context of the Zulip application.  Among other things, this enables session theft. Only deployments which use the S3 storage (not the local-disk storage) are affected, and only deployments which deployed commit 04cf68b45ebb5c03247a0d6453e35ffc175d55da, which has only been in `main`, not any numbered release. Users affected should upgrade from main again to deploy this fix. Switching from S3 storage to the local-disk storage would nominally mitigate this, but is likely more involved than upgrading to the latest `main` which addresses the issue.
Severity: nan (CVSS v3 base score: 4.6)
Weakness: CWE-436 \u2014 Interpretation Conflict
Product A handles inputs or steps differently than Product B, which causes A to perform incorrect actions based on its perception of B's state.
Repository: https://github.com/zulip/zulip
Files changed:
puppet/zulip/files/nginx/zulip-include-frontend/uploads-internal.conf
puppet/zulip/templates/nginx/s3-cache.template.erb
zerver/lib/upload/s3.py
zerver/views/upload.py
Commit message: CVE-2023-22735: Provide the Content-Disposition header from S3.

The Content-Type of user-provided uploads was provided by the browser
at initial upload time, and stored in S3; however, 04cf68b45ebb
switched to determining the Content-Disposition merely from the
filename.  This makes uploads vulnerable to a stored XSS, wherein a
file uploaded with a content-type of `text/html` and an extension of
`.png` would be served to browsers as `Content-Disposition: inline`,
which is unsafe.

The `Content-Security-Policy` headers in the previous commit mitigate
this, but only for browsers which support them.

Revert parts of 04cf68b45ebb, specifically by allowing S3 to provide
the Content-Disposition header, and using the
`ResponseContentDisposition` argument when necessary to override it to
`attachment`.  Because we expect S3 responses to vary based on this
argument, we include it in the cache key; since the query parameter
has dashes in it, we can't use use the helper `$arg_` variables, and
must parse it from the query parameters manually.

Adding the disposition may decrease the cache hit rate somewhat, but
downloads are infrequent enough that it is unlikely to have a
noticeable effect.  We take care to not adjust the cache key for
requests which do not specify the disposition.


Vulnerable code:
    proxy_hide_header Content-Disposition;
    # We are _leaving_ S3 to provide Content-Type and Accept-Ranges
    # headers, which are the two remaining headers which nginx would
    # also pass through from the first response.  Django explicitly
    # unsets the former, and does not set the latter.
    # Don't include query parameters in the cache key, since those
    # include a time-based auth token
    proxy_cache_key $download_url;
def get_signed_upload_url(path: str) -> str:
        Params={
            "Bucket": settings.S3_AUTH_UPLOADS_BUCKET,
            "Key": path,
        },
def serve_s3(request: HttpRequest, path_id: str, download: bool = False) -> HttpResponse:
    url = get_signed_upload_url(path_id)
    patch_disposition_header(response, path_id, download)
def serve_local(request: HttpRequest, path_id: str, download: bool = False) -> HttpResponseBase:
    return serve_file(request, user_profile, realm_id_str, filename, url_only=False, download=True)
    download: bool = False,
    mimetype, encoding = guess_type(path_id)
    download = download or mimetype not in INLINE_MIME_TYPES

        return serve_local(request, path_id, download=download)
        return serve_s3(request, path_id, download=download)
    mimetype, encoding = guess_type(path_id)
    download = mimetype not in INLINE_MIME_TYPES

        return serve_local(request, path_id, download=download)
        return serve_s3(request, path_id, download=download)

Fixed code:
    # We are _leaving_ S3 to provide Content-Type,
    # Content-Disposition, and Accept-Ranges headers, which are the
    # three remaining headers which nginx would also pass through from
    # the first response.  Django explicitly unsets the first, and
    # does not set the latter two.

    # We only include the requested content-disposition in the cache
    # key, so that we cache "Content-Disposition: attachment"
    # separately from the inline version.
    proxy_cache_key $download_url$s3_disposition_cache_key;

# This is used when proxying requests to S3; we wish to know if the
# proxied request is asking to override the Content-Disposition in its
# response, so we can adjust our cache key.  Unfortunately, $arg_foo
# style variables pre-parsed from query parameters don't work with
# query parameters with dashes, so we parse it out by hand.  Despite
# needing to be declared at the 'http' level,. nginx applies maps like
# this lazily, so this only affects internal S3 proxied requests.

map $args $s3_disposition_cache_key {
    default "";
    "~(^|&)(?<param>response-content-disposition=[^&]+)" "?$param";
}
def get_signed_upload_url(path: str, force_download: bool = False) -> str:
    params = {
        "Bucket": settings.S3_AUTH_UPLOADS_BUCKET,
        "Key": path,
    }
    if force_download:
        params["ResponseContentDisposition"] = "attachment"
        Params=params,
def serve_s3(request: HttpRequest, path_id: str, force_download: bool = False) -> HttpResponse:
    url = get_signed_upload_url(path_id, force_download=force_download)

    # It is important that S3 generate both the Content-Type and
    # Content-Disposition headers; when the file was uploaded, we
    # stored the browser-provided value for the former, and set
    # Content-Disposition according to if that was safe.  As such,
    # only S3 knows if a given attachment is safe to inline; we only
    # override Content-Disposition to "attachment", and do so by
    # telling S3 that is what we want in the signed URL.
def serve_local(
    request: HttpRequest, path_id: str, force_download: bool = False
) -> HttpResponseBase:
    mimetype, encoding = guess_type(path_id)
    download = force_download or mimetype not in INLINE_MIME_TYPES

    return serve_file(
        request, user_profile, realm_id_str, filename, url_only=False, force_download=True
    )
    force_download: bool = False,
        return serve_local(request, path_id, force_download=force_download)
        return serve_s3(request, path_id, force_download=force_download)
        return serve_local(request, path_id)
        return serve_s3(request, path_id)