Skip to content

Commit

Permalink
Bug 1406068: Expand the list of DLLs that are suspected of causing a …
Browse files Browse the repository at this point in the history
…crash in ImageBridgeChild::InitForContent. r=jimm

I think that trying to slice this up by feature is just going to lead to complications down the line,
so to keep it simple I've moved this to the launch code for all sandboxed children, not just when the
Alternate Desktop is enabled.
This also, similar to chromium, only adds them to the blocklist if they are loaded in the parent.
  • Loading branch information
bobowen committed Oct 10, 2017
1 parent 4a6ee59 commit ff9470a
Showing 1 changed file with 33 additions and 17 deletions.
50 changes: 33 additions & 17 deletions security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "sandboxBroker.h"

#include <string>
#include <vector>

#include "base/win/windows_version.h"
#include "mozilla/Assertions.h"
Expand All @@ -28,10 +29,26 @@
#include "sandbox/win/src/security_level.h"
#include "WinUtils.h"

// We're just blocking one DLL for the moment because of problems with the
// Alternate Desktop. If and when we expand this we'll make this a static list
// and add checking to see if DLL is loaded in the parent.
#define WEBROOT_DLL L"WRusr.dll"
// This list of DLLs have been found to cause instability in sandboxed child
// processes and so they will be unloaded if they attempt to load.
const std::vector<std::wstring> kDllsToUnload = {
// HitmanPro - SurfRight now part of Sophos (bug 1400637)
L"hmpalert.dll",

// K7 Computing (bug 1400637)
L"k7pswsen.dll",

// Symantec (bug 1400637)
L"prntm64.dll",
L"sysfer.dll",

// Avast Antivirus (bug 1400637)
L"snxhk64.dll",
L"snxhk.dll",

// Webroot SecureAnywhere (bug 1400637)
L"wrusr.dll",
};

namespace mozilla
{
Expand Down Expand Up @@ -216,9 +233,20 @@ SandboxBroker::LaunchApp(const wchar_t *aPath,
sandbox::TargetPolicy::FILES_ALLOW_ANY, logFileName);
}

// Add DLLs to the policy that have been found to cause instability with the
// sandbox, so that they will be unloaded when they attempt to load.
sandbox::ResultCode result;
for (std::wstring dllToUnload : kDllsToUnload) {
// Similar to Chromium, we only add a DLL if it is loaded in this process.
if (::GetModuleHandleW(dllToUnload.c_str())) {
result = mPolicy->AddDllToUnload(dllToUnload.c_str());
MOZ_RELEASE_ASSERT(sandbox::SBOX_ALL_OK == result,
"AddDllToUnload should never fail, what happened?");
}
}

// Ceate the sandboxed process
PROCESS_INFORMATION targetInfo = {0};
sandbox::ResultCode result;
sandbox::ResultCode last_warning = sandbox::SBOX_ALL_OK;
DWORD last_error = ERROR_SUCCESS;
result = sBrokerService->SpawnTarget(aPath, aArguments, mPolicy,
Expand Down Expand Up @@ -445,12 +473,6 @@ SandboxBroker::SetSecurityLevelForContentProcess(int32_t aSandboxLevel,
MOZ_RELEASE_ASSERT(sandbox::SBOX_ALL_OK == result,
"Failed to create alternate desktop for sandbox.");

// Webroot SecureAnywhere causes crashes when we use an Alternate Desktop,
// so block the DLL from loading in the child process. (bug 1400637)
result = mPolicy->AddDllToUnload(WEBROOT_DLL);
MOZ_RELEASE_ASSERT(sandbox::SBOX_ALL_OK == result,
"AddDllToUnload should never fail, what happened?");

mitigations |= sandbox::MITIGATION_IMAGE_LOAD_NO_LOW_LABEL;
// If we're running from a network drive then we can't block loading from
// remote locations.
Expand Down Expand Up @@ -833,12 +855,6 @@ SandboxBroker::SetSecurityLevelForGMPlugin(SandboxLevel aLevel)
SANDBOX_ENSURE_SUCCESS(result,
"Failed to create alternate desktop for sandbox.");

// Webroot SecureAnywhere causes crashes when we use an Alternate Desktop,
// so block the DLL from loading in the child process. (bug 1400637)
result = mPolicy->AddDllToUnload(WEBROOT_DLL);
MOZ_RELEASE_ASSERT(sandbox::SBOX_ALL_OK == result,
"AddDllToUnload should never fail, what happened?");

result = mPolicy->SetIntegrityLevel(sandbox::INTEGRITY_LEVEL_LOW);
MOZ_ASSERT(sandbox::SBOX_ALL_OK == result,
"SetIntegrityLevel should never fail with these arguments, what happened?");
Expand Down

0 comments on commit ff9470a

Please sign in to comment.