From 660d01438a6a29ebd43f592ac7d6df2dad6a6962 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sun, 22 Apr 2018 20:28:18 +0200 Subject: moebius#230: Consider blocking top level window data: URIs (part 3/3 without tests) https://github.com/MoonchildProductions/moebius/pull/230 --- application/palemoon/base/content/nsContextMenu.js | 3 ++- .../palemoon/base/content/utilityOverlay.js | 5 ++++ browser/base/content/nsContextMenu.js | 3 ++- browser/base/content/utilityOverlay.js | 5 ++++ docshell/base/nsDocShell.cpp | 17 +++++++++++- docshell/base/nsDocShell.h | 1 + docshell/base/nsDocShellLoadInfo.cpp | 15 +++++++++++ docshell/base/nsDocShellLoadInfo.h | 1 + docshell/base/nsIDocShell.idl | 3 +++ docshell/base/nsIDocShellLoadInfo.idl | 6 +++++ docshell/base/nsIWebNavigation.idl | 6 +++++ dom/security/nsContentSecurityManager.cpp | 4 +++ dom/security/test/general/browser.ini | 3 +++ .../browser_test_view_image_data_navigation.js | 30 ++++++++++++++++++++++ .../general/file_view_image_data_navigation.html | 12 +++++++++ ipc/glue/BackgroundUtils.cpp | 2 ++ netwerk/base/LoadInfo.cpp | 22 ++++++++++++++++ netwerk/base/LoadInfo.h | 2 ++ netwerk/base/nsILoadInfo.idl | 5 ++++ netwerk/ipc/NeckoChannelParams.ipdlh | 1 + 20 files changed, 143 insertions(+), 3 deletions(-) create mode 100644 dom/security/test/general/browser_test_view_image_data_navigation.js create mode 100644 dom/security/test/general/file_view_image_data_navigation.html diff --git a/application/palemoon/base/content/nsContextMenu.js b/application/palemoon/base/content/nsContextMenu.js index 3d5d40e4c6..830c209986 100644 --- a/application/palemoon/base/content/nsContextMenu.js +++ b/application/palemoon/base/content/nsContextMenu.js @@ -909,7 +909,8 @@ nsContextMenu.prototype = { Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT); let doc = this.target.ownerDocument; openUILink(viewURL, e, { disallowInheritPrincipal: true, - referrerURI: doc.documentURIObject }); + referrerURI: doc.documentURIObject, + forceAllowDataURI: true }); } }, diff --git a/application/palemoon/base/content/utilityOverlay.js b/application/palemoon/base/content/utilityOverlay.js index b1e78d6a95..86cc5cea5a 100644 --- a/application/palemoon/base/content/utilityOverlay.js +++ b/application/palemoon/base/content/utilityOverlay.js @@ -205,6 +205,7 @@ function openUILinkIn(url, where, aAllowThirdPartyFixup, aPostData, aReferrerURI openLinkIn(url, where, params); } +/* eslint-disable complexity */ function openLinkIn(url, where, params) { if (!where || !url) return; @@ -215,6 +216,7 @@ function openLinkIn(url, where, params) { var aCharset = params.charset; var aReferrerURI = params.referrerURI; var aRelatedToCurrent = params.relatedToCurrent; + var aForceAllowDataURI = params.forceAllowDataURI; var aInBackground = params.inBackground; var aDisallowInheritPrincipal = params.disallowInheritPrincipal; var aInitiatingDoc = params.initiatingDoc; @@ -315,6 +317,9 @@ function openLinkIn(url, where, params) { } if (aDisallowInheritPrincipal) flags |= Ci.nsIWebNavigation.LOAD_FLAGS_DISALLOW_INHERIT_OWNER; + if (aForceAllowDataURI) { + flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FORCE_ALLOW_DATA_URI; + } w.gBrowser.loadURIWithFlags(url, flags, aReferrerURI, null, aPostData); break; case "tabshifted": diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js index 8eb9b034f6..ddf6952026 100644 --- a/browser/base/content/nsContextMenu.js +++ b/browser/base/content/nsContextMenu.js @@ -1158,7 +1158,8 @@ nsContextMenu.prototype = { this.browser.contentPrincipal, Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT); openUILink(this.mediaURL, e, { disallowInheritPrincipal: true, - referrerURI: referrerURI }); + referrerURI: referrerURI, + forceAllowDataURI: true }); } }, diff --git a/browser/base/content/utilityOverlay.js b/browser/base/content/utilityOverlay.js index 0b703b6f84..6ceaf773ea 100644 --- a/browser/base/content/utilityOverlay.js +++ b/browser/base/content/utilityOverlay.js @@ -197,6 +197,7 @@ function openUILinkIn(url, where, aAllowThirdPartyFixup, aPostData, aReferrerURI openLinkIn(url, where, params); } +/* eslint-disable complexity */ function openLinkIn(url, where, params) { if (!where || !url) return; @@ -212,6 +213,7 @@ function openLinkIn(url, where, params) { params.referrerPolicy : Ci.nsIHttpChannel.REFERRER_POLICY_DEFAULT); var aRelatedToCurrent = params.relatedToCurrent; var aAllowMixedContent = params.allowMixedContent; + var aForceAllowDataURI = params.forceAllowDataURI; var aInBackground = params.inBackground; var aDisallowInheritPrincipal = params.disallowInheritPrincipal; var aInitiatingDoc = params.initiatingDoc; @@ -378,6 +380,9 @@ function openLinkIn(url, where, params) { if (aIndicateErrorPageLoad) { flags |= Ci.nsIWebNavigation.LOAD_FLAGS_ERROR_LOAD_CHANGES_RV; } + if (aForceAllowDataURI) { + flags |= Ci.nsIWebNavigation.LOAD_FLAGS_FORCE_ALLOW_DATA_URI; + } let {URI_INHERITS_SECURITY_CONTEXT} = Ci.nsIProtocolHandler; if (aForceAboutBlankViewerInCurrent && diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 596bd5d848..f3db4a3cb2 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -1273,6 +1273,7 @@ nsDocShell::LoadURI(nsIURI* aURI, nsCOMPtr shEntry; nsXPIDLString target; nsAutoString srcdoc; + bool forceAllowDataURI = false; nsCOMPtr sourceDocShell; nsCOMPtr baseURI; @@ -1308,6 +1309,7 @@ nsDocShell::LoadURI(nsIURI* aURI, aLoadInfo->GetSrcdocData(srcdoc); aLoadInfo->GetSourceDocShell(getter_AddRefs(sourceDocShell)); aLoadInfo->GetBaseURI(getter_AddRefs(baseURI)); + aLoadInfo->GetForceAllowDataURI(&forceAllowDataURI); } #if defined(DEBUG) @@ -1561,6 +1563,10 @@ nsDocShell::LoadURI(nsIURI* aURI, flags |= INTERNAL_LOAD_FLAGS_IS_SRCDOC; } + if (forceAllowDataURI) { + flags |= INTERNAL_LOAD_FLAGS_FORCE_ALLOW_DATA_URI; + } + return InternalLoad(aURI, originalURI, loadReplace, @@ -4822,6 +4828,9 @@ nsDocShell::LoadURIWithOptions(const char16_t* aURI, } nsAutoPopupStatePusher statePusher(popupState); + bool forceAllowDataURI = + aLoadFlags & LOAD_FLAGS_FORCE_ALLOW_DATA_URI; + // Don't pass certain flags that aren't needed and end up confusing // ConvertLoadTypeToDocShellLoadInfo. We do need to ensure that they are // passed to LoadURI though, since it uses them. @@ -4851,6 +4860,7 @@ nsDocShell::LoadURIWithOptions(const char16_t* aURI, loadInfo->SetReferrerPolicy(aReferrerPolicy); loadInfo->SetHeadersStream(aHeaderStream); loadInfo->SetBaseURI(aBaseURI); + loadInfo->SetForceAllowDataURI(forceAllowDataURI); if (fixupInfo) { nsAutoString searchProvider, keyword; @@ -10083,6 +10093,7 @@ nsDocShell::InternalLoad(nsIURI* aURI, // principal to inherit is: it should be aTriggeringPrincipal. loadInfo->SetPrincipalIsExplicit(true); loadInfo->SetLoadType(ConvertLoadTypeToDocShellLoadInfo(LOAD_LINK)); + loadInfo->SetForceAllowDataURI(aFlags & INTERNAL_LOAD_FLAGS_FORCE_ALLOW_DATA_URI); rv = win->Open(NS_ConvertUTF8toUTF16(spec), aWindowTarget, // window name @@ -10728,7 +10739,9 @@ nsDocShell::InternalLoad(nsIURI* aURI, nsINetworkPredictor::PREDICT_LOAD, this, nullptr); nsCOMPtr req; - rv = DoURILoad(aURI, aOriginalURI, aLoadReplace, loadFromExternal, aReferrer, + rv = DoURILoad(aURI, aOriginalURI, aLoadReplace, loadFromExternal, + (aFlags & INTERNAL_LOAD_FLAGS_FORCE_ALLOW_DATA_URI), + aReferrer, !(aFlags & INTERNAL_LOAD_FLAGS_DONT_SEND_REFERRER), aReferrerPolicy, aTriggeringPrincipal, principalToInherit, aTypeHint, @@ -10809,6 +10822,7 @@ nsDocShell::DoURILoad(nsIURI* aURI, nsIURI* aOriginalURI, bool aLoadReplace, bool aLoadFromExternal, + bool aForceAllowDataURI, nsIURI* aReferrerURI, bool aSendReferrer, uint32_t aReferrerPolicy, @@ -10954,6 +10968,7 @@ nsDocShell::DoURILoad(nsIURI* aURI, loadInfo->SetPrincipalToInherit(aPrincipalToInherit); } loadInfo->SetLoadTriggeredFromExternal(aLoadFromExternal); + loadInfo->SetForceAllowDataURI(aForceAllowDataURI); // We have to do this in case our OriginAttributes are different from the // OriginAttributes of the parent document. Or in case there isn't a diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index 549d7f5405..63a4e3358e 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -370,6 +370,7 @@ protected: nsIURI* aOriginalURI, bool aLoadReplace, bool aLoadFromExternal, + bool aForceAllowDataURI, nsIURI* aReferrer, bool aSendReferrer, uint32_t aReferrerPolicy, diff --git a/docshell/base/nsDocShellLoadInfo.cpp b/docshell/base/nsDocShellLoadInfo.cpp index 7d0034b040..b00e8e3603 100644 --- a/docshell/base/nsDocShellLoadInfo.cpp +++ b/docshell/base/nsDocShellLoadInfo.cpp @@ -15,6 +15,7 @@ nsDocShellLoadInfo::nsDocShellLoadInfo() : mLoadReplace(false) , mInheritPrincipal(false) , mPrincipalIsExplicit(false) + , mForceAllowDataURI(false) , mSendReferrer(true) , mReferrerPolicy(mozilla::net::RP_Default) , mLoadType(nsIDocShellLoadInfo::loadNormal) @@ -126,6 +127,20 @@ nsDocShellLoadInfo::SetPrincipalIsExplicit(bool aPrincipalIsExplicit) return NS_OK; } +NS_IMETHODIMP +nsDocShellLoadInfo::GetForceAllowDataURI(bool* aForceAllowDataURI) +{ + *aForceAllowDataURI = mForceAllowDataURI; + return NS_OK; +} + +NS_IMETHODIMP +nsDocShellLoadInfo::SetForceAllowDataURI(bool aForceAllowDataURI) +{ + mForceAllowDataURI = aForceAllowDataURI; + return NS_OK; +} + NS_IMETHODIMP nsDocShellLoadInfo::GetLoadType(nsDocShellInfoLoadType* aLoadType) { diff --git a/docshell/base/nsDocShellLoadInfo.h b/docshell/base/nsDocShellLoadInfo.h index b7eaed832a..f3ddcca1e6 100644 --- a/docshell/base/nsDocShellLoadInfo.h +++ b/docshell/base/nsDocShellLoadInfo.h @@ -37,6 +37,7 @@ protected: bool mLoadReplace; bool mInheritPrincipal; bool mPrincipalIsExplicit; + bool mForceAllowDataURI; bool mSendReferrer; nsDocShellInfoReferrerPolicy mReferrerPolicy; nsDocShellInfoLoadType mLoadType; diff --git a/docshell/base/nsIDocShell.idl b/docshell/base/nsIDocShell.idl index 8261c45dcb..e34e6adfdf 100644 --- a/docshell/base/nsIDocShell.idl +++ b/docshell/base/nsIDocShell.idl @@ -116,6 +116,9 @@ interface nsIDocShell : nsIDocShellTreeItem const long INTERNAL_LOAD_FLAGS_NO_OPENER = 0x100; + // Whether a top-level data URI navigation is allowed for that load + const long INTERNAL_LOAD_FLAGS_FORCE_ALLOW_DATA_URI = 0x200; + // NB: 0x80 is available. /** diff --git a/docshell/base/nsIDocShellLoadInfo.idl b/docshell/base/nsIDocShellLoadInfo.idl index 113c0a4c17..8804f63a3c 100644 --- a/docshell/base/nsIDocShellLoadInfo.idl +++ b/docshell/base/nsIDocShellLoadInfo.idl @@ -55,6 +55,12 @@ interface nsIDocShellLoadInfo : nsISupports */ attribute boolean principalIsExplicit; + /** + * If this attribute is true, then a top-level navigation + * to a data URI will be allowed. + */ + attribute boolean forceAllowDataURI; + /* these are load type enums... */ const long loadNormal = 0; // Normal Load const long loadNormalReplace = 1; // Normal Load but replaces current history slot diff --git a/docshell/base/nsIWebNavigation.idl b/docshell/base/nsIWebNavigation.idl index 042b1c5479..241d0731c1 100644 --- a/docshell/base/nsIWebNavigation.idl +++ b/docshell/base/nsIWebNavigation.idl @@ -205,6 +205,12 @@ interface nsIWebNavigation : nsISupports */ const unsigned long LOAD_FLAGS_FIXUP_SCHEME_TYPOS = 0x200000; + /** + * Allows a top-level data: navigation to occur. E.g. view-image + * is an explicit user action which should be allowed. + */ + const unsigned long LOAD_FLAGS_FORCE_ALLOW_DATA_URI = 0x400000; + /** * Loads a given URI. This will give priority to loading the requested URI * in the object implementing this interface. If it can't be loaded here diff --git a/dom/security/nsContentSecurityManager.cpp b/dom/security/nsContentSecurityManager.cpp index c987fed67b..9329c558a0 100644 --- a/dom/security/nsContentSecurityManager.cpp +++ b/dom/security/nsContentSecurityManager.cpp @@ -39,6 +39,10 @@ nsContentSecurityManager::AllowTopLevelNavigationToDataURI(nsIChannel* aChannel) if (loadInfo->GetExternalContentPolicyType() != nsIContentPolicy::TYPE_DOCUMENT) { return true; } + if (loadInfo->GetForceAllowDataURI()) { + // if the loadinfo explicitly allows the data URI navigation, let's allow it now + return true; + } nsCOMPtr uri; nsresult rv = NS_GetFinalChannelURI(aChannel, getter_AddRefs(uri)); NS_ENSURE_SUCCESS(rv, true); diff --git a/dom/security/test/general/browser.ini b/dom/security/test/general/browser.ini index 73ae72dddd..b00baa95dc 100644 --- a/dom/security/test/general/browser.ini +++ b/dom/security/test/general/browser.ini @@ -9,3 +9,6 @@ support-files = [browser_test_data_text_csv.js] support-files = file_data_text_csv.html +[browser_test_view_image_data_navigation.js] +support-files = + file_view_image_data_navigation.html diff --git a/dom/security/test/general/browser_test_view_image_data_navigation.js b/dom/security/test/general/browser_test_view_image_data_navigation.js new file mode 100644 index 0000000000..22de358940 --- /dev/null +++ b/dom/security/test/general/browser_test_view_image_data_navigation.js @@ -0,0 +1,30 @@ +"use strict"; + +const TEST_PAGE = getRootDirectory(gTestPath) + "file_view_image_data_navigation.html"; + +add_task(async function test_principal_right_click_open_link_in_new_tab() { + await SpecialPowers.pushPrefEnv({ + "set": [["security.data_uri.block_toplevel_data_uri_navigations", true]], + }); + + await BrowserTestUtils.withNewTab(TEST_PAGE, async function(browser) { + let loadPromise = BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, true); + + // simulate right-click->view-image + BrowserTestUtils.waitForEvent(document, "popupshown", false, event => { + // These are operations that must be executed synchronously with the event. + document.getElementById("context-viewimage").doCommand(); + event.target.hidePopup(); + return true; + }); + BrowserTestUtils.synthesizeMouseAtCenter("#testimage", + { type: "contextmenu", button: 2 }, + gBrowser.selectedBrowser); + await loadPromise; + + await ContentTask.spawn(gBrowser.selectedBrowser, {}, async function() { + ok(content.document.location.toString().startsWith("data:image/svg+xml;"), + "data:image/svg navigation allowed through right-click view-image") + }); + }); +}); diff --git a/dom/security/test/general/file_view_image_data_navigation.html b/dom/security/test/general/file_view_image_data_navigation.html new file mode 100644 index 0000000000..a3f9acfb4d --- /dev/null +++ b/dom/security/test/general/file_view_image_data_navigation.html @@ -0,0 +1,12 @@ + + + + + Bug 1407891: Test navigation for right-click view-image on data:image/svg + + + + + + + diff --git a/ipc/glue/BackgroundUtils.cpp b/ipc/glue/BackgroundUtils.cpp index 4cfbe8758f..e115fa74c7 100644 --- a/ipc/glue/BackgroundUtils.cpp +++ b/ipc/glue/BackgroundUtils.cpp @@ -280,6 +280,7 @@ LoadInfoToLoadInfoArgs(nsILoadInfo *aLoadInfo, aLoadInfo->GetUpgradeInsecureRequests(), aLoadInfo->GetVerifySignedContent(), aLoadInfo->GetEnforceSRI(), + aLoadInfo->GetForceAllowDataURI(), aLoadInfo->GetForceInheritPrincipalDropped(), aLoadInfo->GetInnerWindowID(), aLoadInfo->GetOuterWindowID(), @@ -357,6 +358,7 @@ LoadInfoArgsToLoadInfo(const OptionalLoadInfoArgs& aOptionalLoadInfoArgs, loadInfoArgs.upgradeInsecureRequests(), loadInfoArgs.verifySignedContent(), loadInfoArgs.enforceSRI(), + loadInfoArgs.forceAllowDataURI(), loadInfoArgs.forceInheritPrincipalDropped(), loadInfoArgs.innerWindowID(), loadInfoArgs.outerWindowID(), diff --git a/netwerk/base/LoadInfo.cpp b/netwerk/base/LoadInfo.cpp index 2f10261cb3..9923f6d30c 100644 --- a/netwerk/base/LoadInfo.cpp +++ b/netwerk/base/LoadInfo.cpp @@ -54,6 +54,7 @@ LoadInfo::LoadInfo(nsIPrincipal* aLoadingPrincipal, , mUpgradeInsecureRequests(false) , mVerifySignedContent(false) , mEnforceSRI(false) + , mForceAllowDataURI(false) , mForceInheritPrincipalDropped(false) , mInnerWindowID(0) , mOuterWindowID(0) @@ -227,6 +228,7 @@ LoadInfo::LoadInfo(nsPIDOMWindowOuter* aOuterWindow, , mUpgradeInsecureRequests(false) , mVerifySignedContent(false) , mEnforceSRI(false) + , mForceAllowDataURI(false) , mForceInheritPrincipalDropped(false) , mInnerWindowID(0) , mOuterWindowID(0) @@ -285,6 +287,7 @@ LoadInfo::LoadInfo(const LoadInfo& rhs) , mUpgradeInsecureRequests(rhs.mUpgradeInsecureRequests) , mVerifySignedContent(rhs.mVerifySignedContent) , mEnforceSRI(rhs.mEnforceSRI) + , mForceAllowDataURI(rhs.mForceAllowDataURI) , mForceInheritPrincipalDropped(rhs.mForceInheritPrincipalDropped) , mInnerWindowID(rhs.mInnerWindowID) , mOuterWindowID(rhs.mOuterWindowID) @@ -315,6 +318,7 @@ LoadInfo::LoadInfo(nsIPrincipal* aLoadingPrincipal, bool aUpgradeInsecureRequests, bool aVerifySignedContent, bool aEnforceSRI, + bool aForceAllowDataURI, bool aForceInheritPrincipalDropped, uint64_t aInnerWindowID, uint64_t aOuterWindowID, @@ -341,6 +345,7 @@ LoadInfo::LoadInfo(nsIPrincipal* aLoadingPrincipal, , mUpgradeInsecureRequests(aUpgradeInsecureRequests) , mVerifySignedContent(aVerifySignedContent) , mEnforceSRI(aEnforceSRI) + , mForceAllowDataURI(aForceAllowDataURI) , mForceInheritPrincipalDropped(aForceInheritPrincipalDropped) , mInnerWindowID(aInnerWindowID) , mOuterWindowID(aOuterWindowID) @@ -653,6 +658,23 @@ LoadInfo::GetEnforceSRI(bool* aResult) return NS_OK; } +NS_IMETHODIMP +LoadInfo::SetForceAllowDataURI(bool aForceAllowDataURI) +{ + MOZ_ASSERT(!mForceAllowDataURI || + mInternalContentPolicyType == nsIContentPolicy::TYPE_DOCUMENT, + "can only allow data URI navigation for TYPE_DOCUMENT"); + mForceAllowDataURI = aForceAllowDataURI; + return NS_OK; +} + +NS_IMETHODIMP +LoadInfo::GetForceAllowDataURI(bool* aForceAllowDataURI) +{ + *aForceAllowDataURI = mForceAllowDataURI; + return NS_OK; +} + NS_IMETHODIMP LoadInfo::GetForceInheritPrincipalDropped(bool* aResult) { diff --git a/netwerk/base/LoadInfo.h b/netwerk/base/LoadInfo.h index 99deae2d25..86cfddc683 100644 --- a/netwerk/base/LoadInfo.h +++ b/netwerk/base/LoadInfo.h @@ -94,6 +94,7 @@ private: bool aUpgradeInsecureRequests, bool aVerifySignedContent, bool aEnforceSRI, + bool aForceAllowDataURI, bool aForceInheritPrincipalDropped, uint64_t aInnerWindowID, uint64_t aOuterWindowID, @@ -139,6 +140,7 @@ private: bool mUpgradeInsecureRequests; bool mVerifySignedContent; bool mEnforceSRI; + bool mForceAllowDataURI; bool mForceInheritPrincipalDropped; uint64_t mInnerWindowID; uint64_t mOuterWindowID; diff --git a/netwerk/base/nsILoadInfo.idl b/netwerk/base/nsILoadInfo.idl index 5b5eb425a6..5b35736e5c 100644 --- a/netwerk/base/nsILoadInfo.idl +++ b/netwerk/base/nsILoadInfo.idl @@ -469,6 +469,11 @@ interface nsILoadInfo : nsISupports */ [infallible] attribute boolean enforceSRI; + /** + * If true, toplevel data: URI navigation is allowed + */ + [infallible] attribute boolean forceAllowDataURI; + /** * The SEC_FORCE_INHERIT_PRINCIPAL flag may be dropped when a load info * object is created. Specifically, it will be dropped if the SEC_SANDBOXED diff --git a/netwerk/ipc/NeckoChannelParams.ipdlh b/netwerk/ipc/NeckoChannelParams.ipdlh index e1438caccb..4f4dcf6a9b 100644 --- a/netwerk/ipc/NeckoChannelParams.ipdlh +++ b/netwerk/ipc/NeckoChannelParams.ipdlh @@ -39,6 +39,7 @@ struct LoadInfoArgs bool upgradeInsecureRequests; bool verifySignedContent; bool enforceSRI; + bool forceAllowDataURI; bool forceInheritPrincipalDropped; uint64_t innerWindowID; uint64_t outerWindowID; -- cgit v1.2.3