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

Status: RESOLVED FIXED

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

Description

[Irvan Kurniawan (:sourc7)]

Attachment: ssl-seturl-failure-heap-use-after-free-4.txt

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);

Comment 1

[Irvan Kurniawan (:sourc7)]

Attachment: ssl_seturl.diff

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

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

For more information, please visit BugBot documentation.

Comment 3

[Benjamin Beurdouche [:beurdouche]]

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

Comment 4

[John Schanck [:jschanck]]

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?

Comment 5

[Dana Keeler (she/her) (use needinfo) [:keeler]]

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

Comment 6

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: Bug 1840144 - clean up some TLS socket creation error handling r?jschanck

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a5a183d53f8e
clean up some TLS socket creation error handling r=jschanck

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a5a183d53f8e

Comment 9

[BugBot [:suhaib / :marco/ :calixte]]

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.

Comment 10

[Dana Keeler (she/her) (use needinfo) [:keeler]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Dana Keeler (she/her) (use needinfo) [:keeler]]

Attachment: Bug 1840144 - clean up some TLS socket creation error handling r?jschanck

Original Revision: https://phabricator.services.mozilla.com/D193722

Comment 13

[Phabricator Automation]

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 14

[Dana Keeler (she/her) (use needinfo) [:keeler]]

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.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

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

Approved for 121.0b7.

Comment 16

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/ff6cf7531182

Comment 17

[Ryan VanderMeulen [:RyanVM]]

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

Approved for 115.6esr.

Comment 18

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/dc32e9be8aea

Comment 19

[Jason Kratzer [:jkratzer]]

Attachment: advisory.txt

Comment 20

[Daniel Veditz [:dveditz]]

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