2027310 - Stack-use-after-return in [@ ShowProtectedAuthPrompt] due to early return while background task holds stack references

Status: RESOLVED FIXED

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

Description

[Bugmon [:jkratzer for issues]]

Summary

Stack-use-after-return vulnerability in security/manager/ssl/nsNSSCallbacks.cpp line 500. The ShowProtectedAuthPrompt function dispatches a background task that captures stack-local variables (done, result, slot) by reference via [&]. If FormatValueSync (line 522) or prompt->Alert (line 526) fails, the function returns early without waiting for the background task to complete via SpinEventLoopUntil. The background task then reads the slot pointer and writes result/done on a stack frame that has been freed and potentially reused by other functions.

Affected Code

File: security/manager/ssl/nsNSSCallbacks.cpp, lines 487-541

static char* ShowProtectedAuthPrompt(PK11SlotInfo* slot, nsIPrompt* prompt) {
  ...
  Atomic<bool> done;
  Atomic<SECStatus> result;
  nsresult rv =
      NS_DispatchBackgroundTask(NS_NewRunnableFunction(__func__, [&]() mutable {
        result = PK11_CheckUserPassword(slot, nullptr);  // reads slot, writes result
        done = true;                                      // writes done
      }));
  if (NS_FAILED(rv)) {
    return nullptr;
  }
  ...
  l10n->FormatValueSync(l10nId, l10nArgs, promptString, errorResult);
  if (NS_FAILED(errorResult.StealNSResult())) {
    return nullptr;  // BUG: early return, background task still pending
  }
  rv = prompt->Alert(nullptr, NS_ConvertUTF8toUTF16(promptString).get());
  if (NS_FAILED(rv)) {
    return nullptr;  // BUG: early return, background task still pending
  }

  MOZ_ALWAYS_TRUE(SpinEventLoopUntil(
      "ShowProtectedAuthPrompt"_ns, [&]() { return static_cast<bool>(done); }));
  ...
}

The [&] capture creates references to stack variables. The SpinEventLoopUntil at line 530 is the only synchronization point with the background task, but two early-return paths skip it entirely. With real PIN-pad hardware, PK11_CheckUserPassword (which calls C_Login) blocks for seconds waiting for the user to enter a PIN on the physical keypad — guaranteeing the background task outlives the function on the error paths.

Correct Pattern

static char* ShowProtectedAuthPrompt(PK11SlotInfo* slot, nsIPrompt* prompt) {
  ...
  Atomic<bool> done(false);
  Atomic<SECStatus> result;
  nsresult rv =
      NS_DispatchBackgroundTask(NS_NewRunnableFunction(__func__, [&]() mutable {
        result = PK11_CheckUserPassword(slot, nullptr);
        done = true;
      }));
  if (NS_FAILED(rv)) {
    return nullptr;
  }

  // Always wait for the background task before returning, regardless of
  // errors encountered while showing the prompt.
  auto waitForTask = MakeScopeExit([&]() {
    MOZ_ALWAYS_TRUE(SpinEventLoopUntil(
        "ShowProtectedAuthPrompt"_ns,
        [&]() { return static_cast<bool>(done); }));
  });

  ...
  if (NS_FAILED(errorResult.StealNSResult())) {
    return nullptr;  // waitForTask runs on scope exit
  }
  rv = prompt->Alert(nullptr, NS_ConvertUTF8toUTF16(promptString).get());
  if (NS_FAILED(rv)) {
    return nullptr;  // waitForTask runs on scope exit
  }
  ...
}

Alternatively, heap-allocate the shared state and pass it to the lambda by RefPtr, or move the prompt setup before dispatching the background task so all failure paths occur before dispatch.

Exploit Chain

1. Victim has a smartcard reader with CKF_PROTECTED_AUTHENTICATION_PATH (PIN-pad) installed as a PKCS#11 module in Firefox
2. Victim navigates to attacker-controlled HTTPS server that requests TLS client certificate authentication
3. Firefox enumerates certificates, NSS calls PK11PasswordPrompt callback → PK11PasswordPromptRunnable::RunOnTargetThread → ShowProtectedAuthPrompt on the main thread
4. Background task dispatched; C_Login on the PIN-pad blocks waiting for physical PIN entry
5. FormatValueSync fails (missing/corrupt locale string for protected-auth-alert) or prompt->Alert fails (window destroyed, prompt service error)
6. ShowProtectedAuthPrompt returns nullptr without waiting; stack frame freed
7. Main thread continues executing, reusing the stack for other frames (e.g., Skia rendering)
8. Background thread wakes from C_Login, reads garbage slot pointer and writes result/done into live stack data of an unrelated function → memory corruption in the parent process

Steps to Reproduce

A PKCS#11 module exposing a slot with CKF_PROTECTED_AUTHENTICATION_PATH must be loaded. The provided patch loads libpkcs11testmodule.so (Firefox's test module which has such a slot at index 1), triggers PK11_Authenticate on it, adds a 500ms sleep before PK11_CheckUserPassword to simulate hardware PIN-entry latency, and forces the existing early-return path to simulate prompt failure. Apply the patch, rebuild, and load any page.

Security Impact

Severity: Moderate-to-High. Stack-use-after-return in the parent process with both read (8-byte pointer dereference through PK11_CheckUserPassword) and write (4-byte SECStatus + 1-byte bool) primitives on freed stack memory that may be reused by arbitrary main-thread code.

Attacker capability: A remote attacker controlling an HTTPS server can initiate the code path via client-certificate-authentication requests. Corruption of live main-thread stack data could lead to control-flow hijacking in the parent (chrome) process.

Preconditions: (1) Victim has PIN-pad smartcard hardware loaded as a PKCS#11 module — common in enterprise/government deployments. (2) The error path must trigger: either a missing protected-auth-alert localization string (broken locale pack) or nsIPrompt::Alert failure (window teardown race, headless/automation contexts). The second precondition limits reliability but does not eliminate the risk, and the code defect is unambiguous regardless.

ASAN Report

==124183==ERROR: AddressSanitizer: stack-use-after-return on address 0x7462250d6020 at pc 0x746208b148bd bp 0x7461d13100b0 sp 0x7461d13100a8
READ of size 8 at 0x7462250d6020 thread T10
    #0 0x746208b148bc in ShowProtectedAuthPrompt(PK11SlotInfoStr*, nsIPrompt*)::$_0::operator()() /firefox/security/manager/ssl/nsNSSCallbacks.cpp:504:41
    #1 0x746208b148bc in mozilla::detail::RunnableFunction<ShowProtectedAuthPrompt(PK11SlotInfoStr*, nsIPrompt*)::$_0>::Run() /firefox/obj-x86_64-pc-linux-gnu/dist/include/nsThreadUtils.h:547:5
    #2 0x7461fa62c737 in nsThreadPool::Run() /firefox/xpcom/threads/nsThreadPool.cpp:444:14
    #3 0x7461fa61f77a in nsThread::ProcessNextEvent(bool, bool*) /firefox/xpcom/threads/nsThread.cpp:1173:16
    #4 0x7461fa6282f9 in NS_ProcessNextEvent(nsIThread*, bool) /firefox/xpcom/threads/nsThreadUtils.cpp:465:10
    #5 0x7461fc07a361 in mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) /firefox/ipc/glue/MessagePump.cpp:297:20

Address 0x7462250d6020 is located in stack of thread T0 at offset 32 in frame
    #0 0x7461fe6bb2b7 in SkScan::AAAFillPath(SkPathRaw const&, SkBlitter*, SkIRect const&, SkIRect const&, bool) /firefox/gfx/skia/skia/src/core/SkScan_AAAPath.cpp:1728:0

  This frame has 6 object(s):
    [32, 48) 'sectBounds.i' (line 375) <== Memory access at offset 32 is inside this variable

SUMMARY: AddressSanitizer: stack-use-after-return
Shadow bytes around the buggy address:
=>0x7462250d6000: f5 f5 f5 f5[f5]f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5
  Stack after return:      f5

Suggested Fix

Use MakeScopeExit to guarantee the background task is joined on all return paths after dispatch succeeds:

  nsresult rv =
      NS_DispatchBackgroundTask(NS_NewRunnableFunction(__func__, [&]() mutable {
        result = PK11_CheckUserPassword(slot, nullptr);
        done = true;
      }));
  if (NS_FAILED(rv)) {
    return nullptr;
  }
+ auto joinTask = MakeScopeExit([&]() {
+   MOZ_ALWAYS_TRUE(SpinEventLoopUntil(
+       "ShowProtectedAuthPrompt"_ns,
+       [&]() { return static_cast<bool>(done); }));
+ });

Then remove the standalone SpinEventLoopUntil at line 530. Alternatively, restructure to perform localization and show the alert before dispatching the background task, so all early returns occur before the task exists.

Comment 1

[Bugmon [:jkratzer for issues]]

Attachment: test.html

Comment 2

[Bugmon [:jkratzer for issues]]

Attachment: reproduction_notes.md

Comment 3

[Bugmon [:jkratzer for issues]]

Attachment: source_patch.diff

Comment 4

[Bugmon [:jkratzer for issues]]

Attachment: crash_stack.txt

Comment 6

[Christian Holler (:decoder)]

Attachment: Wait for background task on all return paths via MakeScopeExit

Comment 7

[Christian Holler (:decoder)]

Prefs: none required — code path is not pref-gated. sec-moderate looks correct: the repro patch only makes existing error paths (lines 522-527) and real hardware latency deterministic; it doesn't enable otherwise-unreachable code.

Reproduced with the attached repro patch (adapted module path): ASAN stack-use-after-return at nsNSSCallbacks.cpp:504:41, background thread reading slot from T0's freed stack frame.

Fix: adds a MakeScopeExit guard immediately after dispatch succeeds, so any early return waits for the background task before unwinding the stack. done/result are declared before the guard, so they outlive it during destruction.

Note: comment 0 suggests removing the standalone SpinEventLoopUntil at line 530 — that would be wrong, since switch (result) reads result before the scope-exit guard runs. This patch keeps both; the guard is a no-op on the success path since done is already true.

Verified: with fix + repro patch applied, Firefox runs cleanly with no ASAN errors.

This is the analysis tool's suggested fix. Feel welcome to adopt it as a starting point and evolve it as needed to meet our coding standards.

Comment 8

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

Attachment: (secure)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/245c8a0ee643
https://hg.mozilla.org/integration/autoland/rev/660bf55b2863
improve thread coordination in ShowProtectedAuthPrompt r=jschanck

Comment 10

[Donal Meehan [:dmeehan]]

https://hg-edge.mozilla.org/mozilla-central/rev/660bf55b2863

Comment 11

[BugBot [:suhaib / :marco]]

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.
  • See https://wiki.mozilla.org/Release_Management/Requesting_an_Uplift for documentation on how to request an uplift.
• If no, please set status-firefox150 to wontfix.

For more information, please visit BugBot documentation.

Comment 12

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

This is such a rare configuration that I don't think it's worth uplifting to beta.