1850200 - (CVE-2023-6211) HTTPS-Only mode bypass through clickjacking

Status: RESOLVED FIXED

(Core :: DOM: Security, defect, P3)

Description

[Muneaki Nishimura]

Attachment: https-only-mode-click-jacking-bypass.mov

As well as the TLS certificate exception screen pointed out by CVE-2023-34414, HTTPS-Only mode disable button does not have activation-delay implemented. This could be used for clickjacking with human response time delays.

In the following example site, when the mouse touches the button labeled Click Me, the screen switches to the HTTPS-Only Mode warning page. If the user presses the button even a little late, the button to disable HTTPS-Only Mode is inadvertently pressed, and the page transition to the http: page is succeeded.
https://csrf.jp/2023/https-only/

Comment 1

[Daniel Veditz [:dveditz]]

A big part of bug 1695986 was the ability to gum up the graphics pipeline to make the clickjacking more reliable (the button wasn't drawn until well after it started accepting clicks), but when we were updating aboutNetError.js we should have checked all the other panel types for the same issue.

Comment 2

[Malte Jürgens [:maltejur]]

I can work on this. For the timeout value we should probably use the existing security.dialog_enable_delay, although I feel like one second is a bit excessive and could possibly be an annoyance for some users. Also Dan, can you add me to the CC of the bug you mentioned and that is linked in the CVE? I currently do not have access to that.

Comment 3

[Daniel Veditz [:dveditz]]

Added you to that bug. We should definitely use the standard pref. Arguments about the length of the delay are a separate issue and largely apply the same to all the places. By using the same pref all can be adjusted at once.

I'd argue that using "inert" instead of "disabled" would be better: Users notice when it's greyed out and then changes; with inert it would look exactly the same and the only people who notice (because the click did nothing) would be those who were about to be tricked.

Comment 4

[Malte Jürgens [:maltejur]]

Attachment: Bug 1850200 - Add delay to HTTPS-Only "Continue to HTTPS Site" button r?freddyb

Comment 5

[Malte Jürgens [:maltejur]]

Attachment: Bug 1850200 - Add test for delay on HTTPS-Only "Continue to HTTPS Site" button r?freddyb

Depends on D187887

Comment 6

[Malte Jürgens [:maltejur]]

Okay, I have decided to go with inert then for the disabled button, as I also think that works better. Since both inert and disabled are now used in different places, which is a bit inconsistent, I've opened a follow-up bug 1852949 to change all occurrences to inert.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/32dbea1b0b91
Add delay to HTTPS-Only "Continue to HTTPS Site" button r=freddyb
https://hg.mozilla.org/integration/autoland/rev/0bc9cdf35784
Add test for delay on HTTPS-Only "Continue to HTTPS Site" button r=freddyb

Comment 8

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

Backed out for failing toolkit/components/httpsonlyerror/tests/browser/browser_exception.js:
https://hg.mozilla.org/integration/autoland/rev/49a7e292daad5c6727d745e6910b8bde5c3f59a6

push with failures
failure log

TEST-UNEXPECTED-FAIL | toolkit/components/httpsonlyerror/tests/browser/browser_exception.js | Test timed out -

There are also this TV failure and that other TV failure, please check them before re-landing.

Comment 9

[Frederik Braun [:freddy]]

Flaky tests are annoying.... Maybe we can remove the setTimeout/waits and use MutationObserver instead? :/

Comment 10

[Malte Jürgens [:maltejur]]

Thanks for the tip! I've used MutationObserver to fix browser_exception.js, and simplified browser_continue_button_delay.js so that it only checks wether there is a inert or disabled attribute on the button, and not what happens when you actually click on the button. Can you try to land the patch again Freddy? (if the tests look good to you)

Comment 11

[Frederik Braun [:freddy]]

Maybe I misunderstood, or my explanation wasn't super clear.

I was thinking of using the MutationObserver in /browser_continue_button_delay.js to check for an attribute change (inert set/unset), rather than wait(). That would remove unnecessary test time and give you an immediate callback (where you would then have to measure the elapsed time)

Comment 12

[Malte Jürgens [:maltejur]]

Ah I see, that also makes sense, and sounds like the best way to do it. I'll change the test to do that.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/987b1ef7afa3
Add delay to HTTPS-Only "Continue to HTTPS Site" button r=freddyb
https://hg.mozilla.org/integration/autoland/rev/6da1ce80e10c
Add test for delay on HTTPS-Only "Continue to HTTPS Site" button r=freddyb

Comment 14

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d2252448523
Backed out 2 changesets for causing bc failures on browser_continue_button_delay.js. CLOSED TREE

Comment 15

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

Backed out for causing brower-chrome failures with browser_continue_button_delay.js:
https://hg.mozilla.org/integration/autoland/rev/6d2252448523dec4824297aa68fdada7e0d8e6c9

Push with failures
Failure log

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/62ac1e644213
Add delay to HTTPS-Only "Continue to HTTPS Site" button r=freddyb
https://hg.mozilla.org/integration/autoland/rev/4c5c17a49e40
Add test for delay on HTTPS-Only "Continue to HTTPS Site" button r=freddyb

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/62ac1e644213
https://hg.mozilla.org/mozilla-central/rev/4c5c17a49e40

Comment 18

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

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

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

For more information, please visit BugBot documentation.

Comment 19

[Tom Ritter [:tjr]]

After looking at this closely, we think this is more accurately a low because the attack scenarios require a network vantage point and/or a user-trusted service that operates over HTTP. Additionally, this is an non-default opt-in feature.

Comment 20

[Daniel Veditz [:dveditz]]

Attachment: advisory.txt

Comment 21

[Daniel Veditz [:dveditz]]

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