Closed Bug 1784348 (CVE-2023-29539) Opened 1 year ago Closed 9 months ago

Content-Disposition filename truncation leads to Reflected File Download

Categories

(Core :: Networking: HTTP, defect, P2)

defect

Tracking

()

RESOLVED FIXED
113 Branch
Tracking Status
firefox-esr102 112+ fixed
firefox111 --- wontfix
firefox112 + fixed
firefox113 + fixed

People

(Reporter: trungpaaa, Assigned: smayya)

References

Details

(Keywords: sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main112+][adv-esr102.10+])

Attachments

(2 files)

User-Agent:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:103.0) Gecko/20100101 Firefox/103.0
Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:103.0) Gecko/20100101 Firefox/103.0
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:103.0) Gecko/20100101 Firefox/103.0

When handling filename directive in the Content-Disposition header, the filename would be truncated if the token contains a NULL character and only the first part is kept: filename.exe<NULL>.txt -> filename.exe
This behavior makes Firefox users vulnerable to reflected file download (RFD). For instance, CVE-2022-36359[1] in Django that has been patched[2] is still exploitable on Firefox because the filename truncation provides a trivial way to manipulate and bypass extension checking/appending on the server side. These RFD vulnerabilities such as CVE-2022-36359, CVE-2015-5211[3] and CVE-2020-5398[4] in Spring Frameworks are all rated as High or Important in severity by the project maintainers.
More about RFD: https://securitylab.github.com/research/rfd-spring-mvc-CVE-2020-5398/

I know the vulnerability exists partly because Django allows invalid characters (RFC 2616 I suppose) in the filename. That said, this can only be addressed reliably from the client side. I believe it would be reasonable to stop accepting responses with invalid headers.

from django.shortcuts import render
from django.http import FileResponse

def index(request):
    filename = request.GET.get('filename') + ".pdf"
    print(filename)
    return FileResponse(open("/test.txt", 'rb'), as_attachment=True, filename=filename)

Given request.GET.get('filename') = file.cmd%00, Firefox would download the file file.cmd instead of file.cmd.pdf while Chromium and Safari threw an error (ERR_INVALID_HTTP_RESPONSE) and refused to open the page in this case.

Response:

HTTP/1.1 200 OK
Date: Thu, 11 Aug 2022 19:40:57 GMT
Server: WSGIServer/0.2 CPython/3.8.10
Content-Type: application/octet-stream
Content-Length: 3213
Content-Disposition: attachment; filename="file.cmd<NULL>.pdf"

[1] https://www.djangoproject.com/weblog/2022/aug/03/security-releases/
[2] https://github.com/django/django/commit/bd062445cffd3f6cc6dcd20d13e2abed818fa173
[3] https://tanzu.vmware.com/security/cve-2015-5211
[4] https://tanzu.vmware.com/security/cve-2020-5398

Flags: sec-bounty?
Group: firefox-core-security → core-security
Status: UNCONFIRMED → NEW
Component: Security → Networking: HTTP
Ever confirmed: true
Flags: needinfo?(dveditz)
Product: Firefox → Core
Group: core-security → network-core-security

The server shouldn't be sending these invalid headers, but we shouldn't be accepting them either. But then what? if we ignore the entire Content-Disposition header we could be making the site more vulnerable, because content-disposition can be a defense against hosting "active" web types. Reject the entire response? Delete the null and use the filename assuming it's not there?

Same may apply to other illegal-in-filename characters (especially control characters), but null is definitely a special problem.

Flags: needinfo?(dveditz) → needinfo?(dd.mozilla)
Keywords: sec-moderate
Severity: -- → S3
Priority: -- → P2
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][necko-triaged]

Valentin, can you take a look, I will not have time.

Flags: needinfo?(dd.mozilla) → needinfo?(valentin.gosu)

Chromium and Safari threw an error (ERR_INVALID_HTTP_RESPONSE) and refused to open the page in this case.

I think in this case we should try to align with the other browser engines.

Flags: needinfo?(valentin.gosu)
Whiteboard: [reporter-external] [client-bounty-form] [verif?][necko-triaged] → [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-review]
Whiteboard: [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-review] → [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-queue]
Assignee: nobody → smayya
Attachment #9322073 - Attachment description: WIP: Bug 1784348 - add test to check that null characters in contents disposition headers results in error → WIP: Bug 1784348 - improve checks while parsing MIME parameters
Attachment #9322073 - Attachment description: WIP: Bug 1784348 - improve checks while parsing MIME parameters → Bug 1784348 - improve checks while parsing MIME parameters. r=#necko
Group: network-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → 113 Branch
Regressions: 1822377
Flags: sec-bounty? → sec-bounty+

The patch landed in nightly and beta is affected.
:smayya, is this bug important enough to require an uplift?

  • If yes, please nominate the patch for beta approval.Also, don't forget to request an uplift for the patches in the regression caused by this fix.
  • If no, please set status-firefox112 to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(smayya)
Flags: needinfo?(smayya)
Flags: qe-verify-
Whiteboard: [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-queue] → [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-queue][post-critsmash-triage]

Comment on attachment 9322073 [details]
Bug 1784348 - improve checks while parsing MIME parameters. r=#necko

Beta/Release Uplift Approval Request

  • User impact if declined: Users vulnerable reflected file download
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky):
  • String changes made/needed:
  • Is Android affected?: Yes

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration:
  • User impact if declined: Users vulnerable to reflected file download
  • Fix Landed on Version:
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): The change checks for malformed strings (trailing characters past null) and do not introduce or modify any mime parsing algorithm
Attachment #9322073 - Flags: approval-mozilla-esr102?
Attachment #9322073 - Flags: approval-mozilla-beta?

:Sunil before uplifting this, is there any coordination/action that needs to happen due to the thunderbird regression attached (bug 1822377) ?

Flags: needinfo?(smayya)

I did not understand how this bug has lead to regression of bug 1822377.

Elizabeth, could you please clarify it for me please?
Feel free to guide us on any required coordination for uplift.

Flags: needinfo?(smayya) → needinfo?(elizabeth)

I don't think much needs to happen here other than the TB folks backporting their bustage fix to comm-beta and comm-esr102 after we do the uplift on the Gecko side.

Comment on attachment 9322073 [details]
Bug 1784348 - improve checks while parsing MIME parameters. r=#necko

Approved for 112.0b8

Attachment #9322073 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment on attachment 9322073 [details]
Bug 1784348 - improve checks while parsing MIME parameters. r=#necko

Approved for 102.10esr.

Attachment #9322073 - Flags: approval-mozilla-esr102? → approval-mozilla-esr102+
Flags: needinfo?(elizabeth)
Whiteboard: [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-queue][post-critsmash-triage] → [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main112+]
Whiteboard: [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main112+] → [reporter-external] [client-bounty-form] [verif?][necko-triaged][necko-priority-queue][post-critsmash-triage][adv-main112+][adv-esr102.10+]
Attached file advisory.txt
Alias: CVE-2023-29539
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.