1866006 - Assertion failure: !mFd (NSSSocketControl must outlive its file descriptor!), at security/manager/ssl/NSSSocketControl.h:267

Status: RESOLVED FIXED

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

Description

[Christian Holler (:decoder)]

The attached testcase crashes on mozilla-central revision 20231122-ef0b50d89a7f (build with (metadatabuildFlags not available)).

For detailed crash information, see attachment.

To reproduce the issue, perform the following steps:

1. Download the attached testcase, save as "test.bin".
   2a. Build with --enable-fuzzing (requires Clang and ASan, also build gtests using ./mach gtest dontruntests).
   2b. Alternatively you can download builds from TC using python -mfuzzfetch -a --fuzzing --target firefox gtest (see https://github.com/MozillaSecurity/fuzzfetch).
2. Run FUZZER=NetworkHttp objdir/dist/bin/firefox test.bin

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Christian Holler (:decoder)]

This just started to pop up in the older libFuzzer targets. Did we maybe do a refactoring that broke these? Not sure if this is fuzzing only.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

I'm not aware of any recent changes.
Kershaw, do you know if something changed recently?

Comment 5

[Kershaw Chang [:kershaw]]

That assertion was introduced recently in bug 1840144.
Not sure if this reveals a hidden problem or we can just ignore this check in fuzzing.
Dana, could you take a look? Thanks.

Comment 6

[Andrew McCreight [:mccr8]]

It looks like the assertion is just making sure we don't leak an fd, which doesn't really feel like a security issue.

Comment 8

[Andrew McCreight [:mccr8]]

Looks like this has started showing up in the wild, too. (bug 1866719)

Comment 9

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

Copying crash signatures from duplicate bugs.

Comment 10

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

Attachment: Bug 1866006 - clarify ownership and lifetimes of NSSSocketControl and its associated fd r?jschanck

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62bd80f46c8e
clarify ownership and lifetimes of NSSSocketControl and its associated fd r=jschanck

Comment 12

[Andrew McCreight [:mccr8]]

Is this a 122-only issue? Is this a sec issue? If so what rating do you think it should have? Thanks.

Comment 13

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

This is a 122-only issue because bug 1840144 introduced the assertion that's failing. By itself I don't think it's a security issue (at most, it could be a denial-of-service issue).

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/62bd80f46c8e

Comment 15

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

Attachment: Bug 1866006 - clarify ownership and lifetimes of NSSSocketControl and its associated fd r?jschanck

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

Comment 16

[Phabricator Automation]

Uplift Approval Request

• String changes made/needed: none
• Needs manual QE test: no
• Risk associated with taking this patch: low
• Fix verified in Nightly: yes
• User impact if declined: assertion failures
• Explanation of risk level: This is a small change that is relatively straightforward to verify.
• Steps to reproduce for manual QE testing: n/a
• Code covered by automated testing: yes
• Is Android affected?: yes

Comment 17

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

Comment on attachment 9366573 [details]
Bug 1866006 - clarify ownership and lifetimes of NSSSocketControl and its associated fd r?jschanck

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Release assertion failure.
• User impact if declined: Release assertion failure.
• Fix Landed on Version: 122
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a small change.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9366573 [details]
Bug 1866006 - clarify ownership and lifetimes of NSSSocketControl and its associated fd r?jschanck

Approved for 121.0b7.

Comment 19

[Pulsebot]

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

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9366573 [details]
Bug 1866006 - clarify ownership and lifetimes of NSSSocketControl and its associated fd r?jschanck

Approved for 115.6esr.

Comment 21

[Pulsebot]

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