Closed Bug 1840144 (CVE-2023-6859) Opened 1 year ago Closed 8 months ago

AddressSanitizer: heap-use-after-free on PR_GetIdentitiesLayer after SSL_SetURL PORT_Strdup(url) allocation failure


(Core :: Security: PSM, defect, P1)




122 Branch
Tracking Status
firefox-esr115 121+ fixed
firefox120 --- wontfix
firefox121 + fixed
firefox122 + fixed


(Reporter: sourc7, Assigned: keeler)



(Keywords: csectype-uaf, reporter-external, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [verif?][psm-assigned][adv-main121+][adv-esr115.6+])


(5 files)

When launching Firefox with --disable-e10s and fillMemory code, then when contacting with URL, at rare time Firefox able to crash with AddressSanitizer: heap-use-after-free on PR_GetIdentitiesLayer.

It happen when allocation failure on SSL_SetURL malloc at ss->url = (const char *)PORT_Strdup(url);

Flags: sec-bounty?
Attached patch ssl_seturl.diffSplinter Review
Assignee: nobody → nobody
Group: firefox-core-security → crypto-core-security
Component: Security → Libraries
Product: Firefox → NSS
Version: unspecified → other

The severity field is not set for this bug.
:beurdouche, could you have a look please?

For more information, please visit BugBot documentation.

Flags: needinfo?(bbeurdouche)
Flags: needinfo?(bbeurdouche)
Flags: needinfo?(bbeurdouche)

John, can you please check priority and severity on this when you have a min ?

Severity: -- → S2
Flags: needinfo?(jschanck)
Priority: -- → P2

Priority and severity look right to me.

I think the bug here is in PSM. The MOZ_ASSERT_UNREACHABLE on nsNSSIOLayer.cpp#1602 is reachable in the case of allocation failure. I don't know this code well, but it seems like we might need to destroy plaintextLayer and set it to null in the if (!sslSock) block.

What do you think, Dana?

Flags: needinfo?(jschanck)
Flags: needinfo?(dkeeler)
Flags: needinfo?(bbeurdouche)

Yes, this seems like an issue in PSM, now that I've actually looked at it.

Assignee: nobody → dkeeler
Component: Libraries → Security: PSM
Flags: needinfo?(dkeeler)
Priority: P2 → P1
Product: NSS → Core
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][psm-assigned]
Version: other → unspecified
Pushed by
clean up some TLS socket creation error handling r=jschanck
Group: crypto-core-security → core-security-release
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 122 Branch

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

  • If yes, please nominate the patch for beta approval.
  • If no, please set status-firefox121 to wontfix.

For more information, please visit BugBot documentation.

Flags: needinfo?(dkeeler)

Let's not uplift just yet - bug 1866006 will have to be addressed first.

Blocks: 1866006
No longer blocks: 1866719
Flags: needinfo?(dkeeler)
Flags: sec-bounty? → sec-bounty+

Looks like the fix for bug 1866006 is holding up. Is this ready for Beta and ESR approval requests? Both bugs graft cleanly.

Flags: needinfo?(dkeeler)
Attachment #9366572 - Flags: approval-mozilla-beta?

Uplift Approval Request

  • Fix verified in Nightly: no
  • User impact if declined: UAF under memory pressure (maybe exploitable?)
  • Needs manual QE test: no
  • String changes made/needed: none
  • Risk associated with taking this patch: low-medium
  • Code covered by automated testing: yes
  • Is Android affected?: yes
  • Steps to reproduce for manual QE testing: n/a
  • Explanation of risk level: We did catch one issue with the original patch, but otherwise this is a relatively small change.

Comment on attachment 9366572 [details]
Bug 1840144 - clean up some TLS socket creation error handling r?jschanck

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Potential UAF
  • User impact if declined: This is potentially exploitable. Otherwise, it would just be memory corruption under memory pressure.
  • Fix Landed on Version: 122
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): We already discovered one issue (bug 1866006), but other than that, it's a fairly small change.
Flags: needinfo?(dkeeler)
Attachment #9366572 - Flags: approval-mozilla-esr115?

Comment on attachment 9366572 [details]
Bug 1840144 - clean up some TLS socket creation error handling r?jschanck

Approved for 121.0b7.

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

Comment on attachment 9366572 [details]
Bug 1840144 - clean up some TLS socket creation error handling r?jschanck

Approved for 115.6esr.

Attachment #9366572 - Flags: approval-mozilla-esr115? → approval-mozilla-esr115+
QA Whiteboard: [post-critsmash-triage]
Flags: qe-verify-
Whiteboard: [reporter-external] [client-bounty-form] [verif?][psm-assigned] → [reporter-external] [client-bounty-form] [verif?][psm-assigned][adv-main121+]
Whiteboard: [reporter-external] [client-bounty-form] [verif?][psm-assigned][adv-main121+] → [reporter-external] [client-bounty-form] [verif?][psm-assigned][adv-main121+][adv-esr115.6+]
Attached file advisory.txt
Alias: CVE-2023-6859

Bulk-unhiding security bugs fixed in Firefox 119-121 (Fall 2023). Use "moo-doctrine-subsidy" to filter

Group: core-security-release
You need to log in before you can comment on or make changes to this bug.