Skip to content

Commit

Permalink
Backed out 4 changesets (bug 1735746) for causing failures at browser…
Browse files Browse the repository at this point in the history
…_protocol_custom_sandbox.js. CLOSED TREE

Backed out changeset 703dfd92c775 (bug 1735746)
Backed out changeset 3b06ed08d93b (bug 1735746)
Backed out changeset 9968278b9efe (bug 1735746)
Backed out changeset 49f2e283115d (bug 1735746)
  • Loading branch information
Butkovits Atila committed Apr 5, 2022
1 parent ffbb69e commit 393fc50
Show file tree
Hide file tree
Showing 18 changed files with 14 additions and 536 deletions.
9 changes: 3 additions & 6 deletions docshell/base/nsDocShell.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12974,12 +12974,9 @@ nsresult nsDocShell::OnLinkClickSync(nsIContent* aContent,
nsresult rv =
extProtService->IsExposedProtocol(scheme.get(), &isExposed);
if (NS_SUCCEEDED(rv) && !isExposed) {
return extProtService->LoadURI(
aLoadState->URI(), triggeringPrincipal, nullptr, mBrowsingContext,
/* aTriggeredExternally */
false,
/* aHasValidUserGestureActivation */
aContent->OwnerDoc()->HasValidTransientUserGestureActivation());
return extProtService->LoadURI(aLoadState->URI(), triggeringPrincipal,
nullptr, mBrowsingContext,
/* aTriggeredExternally */ false);
}
}
}
Expand Down
3 changes: 0 additions & 3 deletions dom/base/IframeSandboxKeywordList.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,3 @@ SANDBOX_KEYWORD("allow-presentation", allowpresentation, SANDBOXED_PRESENTATION)
SANDBOX_KEYWORD("allow-storage-access-by-user-activation",
allowstorageaccessbyuseractivatetion, SANDBOXED_STORAGE_ACCESS)
SANDBOX_KEYWORD("allow-downloads", allowdownloads, SANDBOXED_ALLOW_DOWNLOADS)
SANDBOX_KEYWORD("allow-top-navigation-to-custom-protocols",
allowtopnavigationcustomprotocols,
SANDBOXED_TOPLEVEL_NAVIGATION_CUSTOM_PROTOCOLS)
5 changes: 0 additions & 5 deletions dom/base/nsSandboxFlags.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,5 @@ const unsigned long SANDBOXED_TOPLEVEL_NAVIGATION_USER_ACTIVATION = 0x20000;
*/
const unsigned long SANDBOXED_ALLOW_DOWNLOADS = 0x10000;

/**
* This flag prevents content from navigating to custom protocols.
*/
const unsigned long SANDBOXED_TOPLEVEL_NAVIGATION_CUSTOM_PROTOCOLS = 0x40000;

const unsigned long SANDBOX_ALL_FLAGS = 0xFFFFF;
#endif
5 changes: 2 additions & 3 deletions dom/ipc/ContentParent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4487,7 +4487,7 @@ mozilla::ipc::IPCResult ContentParent::RecvLoadURIExternal(
nsIURI* uri, nsIPrincipal* aTriggeringPrincipal,
nsIPrincipal* aRedirectPrincipal,
const MaybeDiscarded<BrowsingContext>& aContext,
bool aWasExternallyTriggered, bool aHasValidUserGestureActivation) {
bool aWasExternallyTriggered) {
if (aContext.IsDiscarded()) {
return IPC_OK();
}
Expand All @@ -4504,8 +4504,7 @@ mozilla::ipc::IPCResult ContentParent::RecvLoadURIExternal(

BrowsingContext* bc = aContext.get();
extProtService->LoadURI(uri, aTriggeringPrincipal, aRedirectPrincipal, bc,
aWasExternallyTriggered,
aHasValidUserGestureActivation);
aWasExternallyTriggered);
return IPC_OK();
}

Expand Down
2 changes: 1 addition & 1 deletion dom/ipc/ContentParent.h
Original file line number Diff line number Diff line change
Expand Up @@ -1066,7 +1066,7 @@ class ContentParent final
nsIURI* uri, nsIPrincipal* triggeringPrincipal,
nsIPrincipal* redirectPrincipal,
const MaybeDiscarded<BrowsingContext>& aContext,
bool aWasExternallyTriggered, bool aHasValidUserGestureActivation);
bool aWasExternallyTriggered);
mozilla::ipc::IPCResult RecvExtProtocolChannelConnectParent(
const uint64_t& registrarId);

Expand Down
3 changes: 1 addition & 2 deletions dom/ipc/PContent.ipdl
Original file line number Diff line number Diff line change
Expand Up @@ -1125,8 +1125,7 @@ parent:
nsIPrincipal triggeringPrincipal,
nsIPrincipal redirectPrincipal,
MaybeDiscardedBrowsingContext browsingContext,
bool wasExternallyTriggered,
bool hasValidUserGestureActivation);
bool wasExternallyTriggered);
async ExtProtocolChannelConnectParent(uint64_t registrarId);

// PrefService message
Expand Down
3 changes: 0 additions & 3 deletions dom/locales/en-US/chrome/security/security.properties
Original file line number Diff line number Diff line change
Expand Up @@ -153,9 +153,6 @@ HTTPSOnlyUpgradeSpeculativeConnection = Upgrading insecure speculative TCP conne
# LOCALIZATION NOTE: %S is the URL of the blocked request;
IframeSandboxBlockedDownload = Download of “%S” was blocked because the triggering iframe has the sandbox flag set.

# LOCALIZATION NOTE: %S is the URL of the blocked request;
SandboxBlockedCustomProtocols = Blocked navigation to custom protocol “%S” from a sandboxed context.

# Sanitizer API
# LOCALIZATION NOTE: Please do not localize "DocumentFragment". It's the name of an API.
SanitizerRcvdNoInput = Received empty or no input. Returning an empty DocumentFragment.
Expand Down
6 changes: 0 additions & 6 deletions modules/libpref/init/StaticPrefList.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1910,12 +1910,6 @@
value: true
mirror: always

# Block sandboxed BrowsingContexts from navigating to external protocols.
- name: dom.block_external_protocol_navigation_from_sandbox
type: bool
value: @IS_NIGHTLY_BUILD@
mirror: always

# Block Insecure downloads from Secure Origins
- name: dom.block_download_insecure
type: bool
Expand Down
72 changes: 2 additions & 70 deletions uriloader/exthandler/nsExternalHelperAppService.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "mozilla/dom/CanonicalBrowsingContext.h"
#include "mozilla/dom/WindowGlobalParent.h"
#include "mozilla/RandomNum.h"
#include "mozilla/StaticPrefs_dom.h"
#include "mozilla/StaticPrefs_security.h"
#include "mozilla/StaticPtr.h"
#include "nsXULAppAPI.h"
Expand All @@ -31,7 +30,6 @@
#include "nsAppDirectoryServiceDefs.h"
#include "nsICategoryManager.h"
#include "nsDependentSubstring.h"
#include "nsSandboxFlags.h"
#include "nsString.h"
#include "nsUnicharUtils.h"
#include "nsIStringEnumerator.h"
Expand Down Expand Up @@ -1097,84 +1095,18 @@ nsresult nsExternalHelperAppService::EscapeURI(nsIURI* aURI, nsIURI** aResult) {
return ios->NewURI(escapedSpec, nullptr, nullptr, aResult);
}

bool ExternalProtocolIsBlockedBySandbox(
BrowsingContext* aBrowsingContext,
const bool aHasValidUserGestureActivation) {
if (!StaticPrefs::dom_block_external_protocol_navigation_from_sandbox()) {
return false;
}

if (!aBrowsingContext || aBrowsingContext->IsTop()) {
return false;
}

uint32_t sandboxFlags = aBrowsingContext->GetSandboxFlags();

if (sandboxFlags == SANDBOXED_NONE) {
return false;
}

if (!(sandboxFlags & SANDBOXED_AUXILIARY_NAVIGATION)) {
return false;
}

if (!(sandboxFlags & SANDBOXED_TOPLEVEL_NAVIGATION)) {
return false;
}

if (!(sandboxFlags & SANDBOXED_TOPLEVEL_NAVIGATION_CUSTOM_PROTOCOLS)) {
return false;
}

if (!(sandboxFlags & SANDBOXED_TOPLEVEL_NAVIGATION_USER_ACTIVATION) &&
aHasValidUserGestureActivation) {
return false;
}

return true;
}

NS_IMETHODIMP
nsExternalHelperAppService::LoadURI(nsIURI* aURI,
nsIPrincipal* aTriggeringPrincipal,
nsIPrincipal* aRedirectPrincipal,
BrowsingContext* aBrowsingContext,
bool aTriggeredExternally,
bool aHasValidUserGestureActivation) {
bool aTriggeredExternally) {
NS_ENSURE_ARG_POINTER(aURI);

if (XRE_IsContentProcess()) {
mozilla::dom::ContentChild::GetSingleton()->SendLoadURIExternal(
aURI, aTriggeringPrincipal, aRedirectPrincipal, aBrowsingContext,
aTriggeredExternally, aHasValidUserGestureActivation);
return NS_OK;
}

// Prevent sandboxed BrowsingContexts from navigating to external protocols.
// This only uses the sandbox flags of the target BrowsingContext of the
// load. The navigating document's CSP sandbox flags do not apply.
if (aBrowsingContext &&
ExternalProtocolIsBlockedBySandbox(aBrowsingContext,
aHasValidUserGestureActivation)) {
// Log an error to the web console of the sandboxed BrowsingContext.
nsAutoString localizedMsg;
nsAutoCString spec;
aURI->GetSpec(spec);

AutoTArray<nsString, 1> params = {NS_ConvertUTF8toUTF16(spec)};
nsresult rv = nsContentUtils::FormatLocalizedString(
nsContentUtils::eSECURITY_PROPERTIES, "SandboxBlockedCustomProtocols",
params, localizedMsg);
NS_ENSURE_SUCCESS(rv, rv);

WindowContext* windowContext = aBrowsingContext->GetCurrentWindowContext();
NS_ENSURE_TRUE(windowContext, NS_ERROR_FAILURE);

nsContentUtils::ReportToConsoleByWindowID(
localizedMsg, nsIScriptError::errorFlag, "Security"_ns,
windowContext->InnerWindowId(),
windowContext->Canonical()->GetDocumentURI());

aTriggeredExternally);
return NS_OK;
}

Expand Down
3 changes: 1 addition & 2 deletions uriloader/exthandler/nsExternalHelperAppService.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ class nsExternalHelperAppService : public nsIExternalHelperAppService,
NS_IMETHOD LoadURI(nsIURI* aURI, nsIPrincipal* aTriggeringPrincipal,
nsIPrincipal* aRedirectPrincipal,
mozilla::dom::BrowsingContext* aBrowsingContext,
bool aWasTriggeredExternally,
bool aHasValidUserGestureActivation) override;
bool aWasTriggeredExternally) override;
NS_IMETHOD SetProtocolHandlerDefaults(nsIHandlerInfo* aHandlerInfo,
bool aOSHandlerExists) override;

Expand Down
6 changes: 3 additions & 3 deletions uriloader/exthandler/nsExternalProtocolHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,9 @@ nsresult nsExtProtocolChannel::OpenURL() {
mLoadInfo->RedirectChain().LastElement()->GetPrincipal(
getter_AddRefs(redirectPrincipal));
}
rv = extProtService->LoadURI(mUrl, triggeringPrincipal, redirectPrincipal,
ctx, mLoadInfo->GetLoadTriggeredFromExternal(),
mLoadInfo->GetHasValidUserGestureActivation());
rv =
extProtService->LoadURI(mUrl, triggeringPrincipal, redirectPrincipal,
ctx, mLoadInfo->GetLoadTriggeredFromExternal());

if (NS_SUCCEEDED(rv) && mListener) {
mStatus = NS_ERROR_NO_CONTENT;
Expand Down
7 changes: 1 addition & 6 deletions uriloader/exthandler/nsIExternalProtocolService.idl
Original file line number Diff line number Diff line change
Expand Up @@ -117,10 +117,6 @@ interface nsIExternalProtocolService : nsISupports
* @param aWasTriggeredExternally
* If true, indicates the load was initiated by an external app.
*
* @param aHasValidUserGestureActivation
* Whether the document that triggered the load had user activation.
* Used for sandbox checks.
*
* @note Embedders that do not expose the http protocol should not currently
* use web-based protocol handlers, as handoff won't work correctly
* (bug 394479).
Expand All @@ -129,8 +125,7 @@ interface nsIExternalProtocolService : nsISupports
[optional] in nsIPrincipal aTriggeringPrincipal,
[optional] in nsIPrincipal aRedirectPrincipal,
[optional] in BrowsingContext aBrowsingContext,
[optional] in bool aWasTriggeredExternally,
[optional] in bool aHasValidUserGestureActivation);
[optional] in bool aWasTriggeredExternally);

/**
* Gets a human-readable description for the application responsible for
Expand Down
6 changes: 0 additions & 6 deletions uriloader/exthandler/tests/mochitest/browser.ini
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,6 @@ support-files =
[browser_protocol_ask_dialog.js]
support-files =
file_nested_protocol_request.html
[browser_protocol_custom_sandbox.js]
support-files =
protocol_custom_sandbox_helper.sjs
[browser_protocol_custom_sandbox_csp.js]
support-files =
protocol_custom_sandbox_helper.sjs
[browser_first_prompt_not_blocked_without_user_interaction.js]
support-files =
file_external_protocol_iframe.html
Expand Down

This file was deleted.

Loading

0 comments on commit 393fc50

Please sign in to comment.