1872995 - Prespawn CIG is not active in Nightly with an empty dynamic DLL blocklist and no IAT injection

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect, P1)

Description

[Yannis Juglaret [:yannis]]

If we look at old patches like this one it seems rather clear that the intention behind prespawn CIG was that it should always be active (in Nightly) as long as the shared section was set up correctly. However, in the current code base, prespawn CIG is only active if we encountered an IAT injected module, or if we added at least one dynamic blocklist entry (which is always the case in tests, but not for most users).

This change in behavior would have been introduced by bug 1744362 part 6. SharedSection::Layout::GetDependentModules returns an empty Span if mDependentModulePathArrayStart is zero:

Span<wchar_t> SharedSection::Layout::GetDependentModules() {
  if (!mDependentModulePathArrayStart) {
    return nullptr;
  }
  return Span<wchar_t>(
      reinterpret_cast<wchar_t*>(reinterpret_cast<uintptr_t>(this) +
                                 mDependentModulePathArrayStart),
      (kSharedViewSize - mDependentModulePathArrayStart) / sizeof(wchar_t));
}

But mDependentModulePathArrayStart is not only zero under bad initialization, when things go right it stays zero unless we add a dependent module or a blocklist entry:

LauncherVoidResult SharedSection::Init() {
  /* ... */
  Layout* view = writableView.as<Layout>();
  view->mK32Exports.Init();
  view->mState = Layout::State::kInitialized;
  // Leave view->mDependentModulePathArrayStart to be zero to indicate
  // we can add blocklist entries
  return Ok();
}

Therefore mDependentModulePathArrayStart cannot and should not be used to discriminate good from bad init. This currently makes us return an empty span in this case which is later interpreted as though there was a problem with the shared section setup:

// If this returns Nothing(), it means the retrieval of the modules failed
// (e.g. when the launcher process is disabled), so the process should not
// enable pre-spawn CIG.
static const Maybe<Vector<const wchar_t*>>& GetPrespawnCigExceptionModules() {
    /* ... */
    Span<const wchar_t> dependentModules = sharedSection->GetDependentModules();
    if (dependentModules.IsEmpty()) {
      return Nothing();
    }
    /* ... */
}

This case (no IAT injection, empty dynamic blocklist) is actually the one encountered by the vast majority of Nightly users. So if we want to restore previous behavior, we have to be very careful about that, because this issue has left prespawn CIG disabled since Nightly 110 for most users so we don't know for sure how our code base could react if we re-enable it. Fortunately our tests do run with prespawn CIG (because they always have a non-empty dynamic blocklist), which should mitigate the risk here.

Comment 1

[Yannis Juglaret [:yannis]]

This also makes me wonder if we should always add the test entry to the blocklist but just hide it in the about:third-party view, to ensure that tests are coherent with users' setup. This part of the issue was introduced by bug 1809513.

Comment 2

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

Set release status flags based on info from the regressing bug 1744362

:gstoll, since you are the author of the regressor, bug 1744362, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 3

[Yannis Juglaret [:yannis]]

This only impacts Nightly builds. Prespawn CIG is intentionally not active in other channels.

Comment 4

[Greg Stoll :gstoll]

Sigh, nice catch.

I'm not thrilled with the idea of always adding the test entry to the blocklist because we don't want to actually block it for Nightly users (although I suppose finding a DLL named "TestDllBlocklist_UserBlocked.dll".

Comment 5

[Greg Stoll :gstoll]

Attachment: Bug 1872995 - Make prespawn CIG work again on Nightly if there are no entries on dynamic blocklist r=yjuglaret

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/70442a08a03f
Make prespawn CIG work again on Nightly if there are no entries on dynamic blocklist r=yjuglaret,win-reviewers,mhowell,bobowen

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/70442a08a03f