1769572 - Firefox crashes if a request is resent from the devtools with a CONNECT method.

Status: VERIFIED FIXED

(DevTools :: Netmonitor, defect, P2)

Description

[Jaxca]

I tried this on a Linux system with Firefox and TorBrowser.

OS: Ubuntu 20.04.4 LTS
Firefox: v100.0
TorBrowser: v91.9.0esr

Steps to reproduce:

1. navigate to any page
2. open the devtools network tab
3. right click on the first GET request
4. select "Edit and resend"
5. change the method from GET to CONNECT
6. click "Send"

At this point it completely crashes the whole browser for some reason.
I tried all the other http methods too but only CONNECT will crash it.
I even tried to break the method name like CONNEC and CONNECTT,
however only the valid CONNECT seems to do the trick. Clearly a nasty
bug there.

Comment 1

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

https://crash-stats.mozilla.org/report/index/312ab54f-604b-424f-8b86-e436a0220516

Comment 2

[Dragana Damjanovic [:dragana]]

CONNECT method should be forbidden in devTools. CONNECT has a special meaning and necko assumes the request is created by necko for a proxy or websocket.

Comment 3

[:Gijs (he/him)]

(In reply to Dragana Damjanovic [:dragana] from comment #2)

CONNECT method should be forbidden in devTools. CONNECT has a special meaning and necko assumes the request is created by necko for a proxy or websocket.

Can we make SetRequestMethod just throw if you try setting it to CONNECT ? I'm worried that even if we fix this here there will be other cases where we allow for this method.

I'm not convinced it's useful to keep this bug security-sensitive/secret - it looks like the crash is a near-null crash, and this requires so much initiative on the user's side that I don't see how a malicious attacker would exploit it anyway.

Comment 4

[Jaxca]

At least it doesn't crash if I run the same CONNECT query from the devtools console with javascript and fetch.
There it was displaying a correct looking error message.

The http method in the "Edit and resend" form could also be a predefined list instead of the text input field imo.

Comment 5

[Daniel Veditz [:dveditz]]

This doesn't look particularly exploitable (a null deref with a fixed offset) and doesn't seem all that likely you could socially-engineer someone into doing this.

Comment 6

[Hubert Boma Manilla (:bomsy)]

Thanks for reporting!

I can reproduce the issue with CONNECT.

(In reply to Jaxca from comment #4)

The http method in the "Edit and resend" form could also be a predefined list instead of the text input field imo.

Yes i agree.
We have a new Edit and Resend panel currently behind a flag devtools.netmonitor.features.newEditAndResend where the METHOD is a list and not editable. We should enable by default soon .

(In reply to :Gijs (he/him) from comment #3)

(In reply to Dragana Damjanovic [:dragana] from comment #2)

CONNECT method should be forbidden in devTools. CONNECT has a special meaning and necko assumes the request is created by necko for a proxy or websocket.

Can we make SetRequestMethod just throw if you try setting it to CONNECT ? I'm worried that even if we fix this here there will be other cases where we allow for this method.

I agree! this should fix the current Edit and Resend panel .We would also remove connect from https://searchfox.org/mozilla-central/rev/7f729f601c0b738f870ae0ed49098f9268e250f9/devtools/client/netmonitor/src/components/new-request/HTTPCustomRequestPanel.js#76 for the new Edit and Resend panel.

Comment 7

[Hubert Boma Manilla (:bomsy)]

Adding an STR for the new Edit and Resend panel which repros the issue.

1. Set devtools.netmonitor.features.newEditAndResend to true
2. Navigate to https://blackbox-example.glitch.me/
3. Open the devtools network tab
4. Right click on the first GET request (with a 304 status)
5. Select "Resend"
6. Select CONNECT method from the list
7. Click Send

ER
Does not crash firefox

AR
Crashes firefox

Comment 8

[Hubert Boma Manilla (:bomsy)]

Attachment: Bug 1769572 - [devtools] The CONNECT method should no longer be sent on request resend r=nchevobbe

The CONNECT method sent on Edit & Resend of certain requests crashes firefox
based on https://bugzilla.mozilla.org/show_bug.cgi?id=1769572#c2, Necko uses CONNECT in a special way, so lets restrict
the devtools from selecting and sending it.

The crash happens with both the current and new edit and resend panels, so this patch
needs to be tested for both.

Use the devtools.netmonitor.features.newEditAndResend and set it to true to enable the new edit and resend panel.

Note: The new edit and resend panel is enabled by default in Nightly.

See https://bugzilla.mozilla.org/show_bug.cgi?id=1769572#c0 for the STR for the current Edit and Resend panel
See https://bugzilla.mozilla.org/show_bug.cgi?id=1769572#c7 for the STR for the new Edit and Resend

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4a7323a316e
[devtools] The CONNECT method should no longer be sent on request resend r=nchevobbe

Comment 10

[Norisz Fay [:noriszfay]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Seems like a pretty low-risk uplift if you want to nominate it for Beta and ESR approval.

Comment 12

[Hubert Boma Manilla (:bomsy)]

Comment on attachment 9277315 [details]
Bug 1769572 - [devtools] The CONNECT method should no longer be sent on request resend r=nchevobbe

Beta/Release Uplift Approval Request

• User impact if declined: Crashes Firefox for developers
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• 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): Small Javascript change. Remove's the CONNECT from the list for the new edit and resend and stop sending CONNECT request for the old panel.
• String changes made/needed:
• Is Android affected?: No

Comment 13

[Hubert Boma Manilla (:bomsy)]

Comment on attachment 9277315 [details]
Bug 1769572 - [devtools] The CONNECT method should no longer be sent on request resend r=nchevobbe

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Crashes firefox for developers.
• User impact if declined: Crashes firefox for developers.
• Fix Landed on Version: 102
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small JS change, removes CONNECT from being sent by devtools

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9277315 [details]
Bug 1769572 - [devtools] The CONNECT method should no longer be sent on request resend r=nchevobbe

Approved for 101.0rc1 and 91.10esr.

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/ec18cb3ff9a7

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/330c54f6b547

Comment 17

[Peter Magyari (Desktop QA)]

Hi,

I've reproduced the issue using Firefox Beta 101.0b9 (20220519220322) and verified the fix on all of the following Firefox versions:

• Firefox Nightly 102.0a1 (20220522190601)
• Firefox Beta 101.0 (20220523135024)
• Firefox 91.10.0esr (20220520202401)

Comment 18

[:Gijs (he/him)]

All crashes are nasty.

Comment 19

[Jaxca]

Good work guys! That input field will have a special meaning for me from now on :)