Bug 1427726 - remove nsIJARChannel::isUnsafe and nsIDocShell::channelIsUnsafe, r?bz draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Wed, 07 Feb 2018 14:25:33 +0000
changeset 780416 21c79896aaf7b27eab2e6e42ce7b897256a8d357
parent 780415 482b3eb780f22d1c2913f2e2ee050e5960851d7e
child 780417 28c995c351ae7b7d02b129a72adf1b152e4e632e
push id105989
push userbmo:gijskruitbosch+bugs@gmail.com
push dateWed, 11 Apr 2018 11:22:19 +0000
reviewersbz
bugs1427726
milestone61.0a1
Bug 1427726 - remove nsIJARChannel::isUnsafe and nsIDocShell::channelIsUnsafe, r?bz MozReview-Commit-ID: 7XyRqzKDbsq
docshell/base/nsDocShell.cpp
docshell/base/nsIDocShell.idl
dom/base/nsGlobalWindowOuter.cpp
modules/libjar/nsIJARChannel.idl
modules/libjar/nsJARChannel.cpp
modules/libjar/nsJARChannel.h
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -1605,34 +1605,16 @@ nsDocShell::GetParentCharset(const Encod
                              nsIPrincipal** aPrincipal)
 {
   aCharset = mParentCharset;
   *aCharsetSource = mParentCharsetSource;
   NS_IF_ADDREF(*aPrincipal = mParentCharsetPrincipal);
 }
 
 NS_IMETHODIMP
-nsDocShell::GetChannelIsUnsafe(bool* aUnsafe)
-{
-  *aUnsafe = false;
-
-  nsIChannel* channel = GetCurrentDocChannel();
-  if (!channel) {
-    return NS_OK;
-  }
-
-  nsCOMPtr<nsIJARChannel> jarChannel = do_QueryInterface(channel);
-  if (!jarChannel) {
-    return NS_OK;
-  }
-
-  return jarChannel->GetIsUnsafe(aUnsafe);
-}
-
-NS_IMETHODIMP
 nsDocShell::GetHasMixedActiveContentLoaded(bool* aHasMixedActiveContentLoaded)
 {
   nsCOMPtr<nsIDocument> doc(GetDocument());
   *aHasMixedActiveContentLoaded = doc && doc->GetHasMixedActiveContentLoaded();
   return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -1680,22 +1662,16 @@ nsDocShell::GetHasTrackingContentLoaded(
 }
 
 NS_IMETHODIMP
 nsDocShell::GetAllowPlugins(bool* aAllowPlugins)
 {
   NS_ENSURE_ARG_POINTER(aAllowPlugins);
 
   *aAllowPlugins = mAllowPlugins;
-  if (!mAllowPlugins) {
-    return NS_OK;
-  }
-
-  bool unsafe;
-  *aAllowPlugins = NS_SUCCEEDED(GetChannelIsUnsafe(&unsafe)) && !unsafe;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::SetAllowPlugins(bool aAllowPlugins)
 {
   mAllowPlugins = aAllowPlugins;
   // XXX should enable or disable a plugin host
@@ -1899,22 +1875,16 @@ nsDocShell::NotifyReflowObservers(bool a
 }
 
 NS_IMETHODIMP
 nsDocShell::GetAllowMetaRedirects(bool* aReturn)
 {
   NS_ENSURE_ARG_POINTER(aReturn);
 
   *aReturn = mAllowMetaRedirects;
-  if (!mAllowMetaRedirects) {
-    return NS_OK;
-  }
-
-  bool unsafe;
-  *aReturn = NS_SUCCEEDED(GetChannelIsUnsafe(&unsafe)) && !unsafe;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsDocShell::SetAllowMetaRedirects(bool aValue)
 {
   mAllowMetaRedirects = aValue;
   return NS_OK;
@@ -9576,44 +9546,16 @@ nsDocShell::InternalLoad(nsIURI* aURI,
         (aFlags & INTERNAL_LOAD_FLAGS_INHERIT_PRINCIPAL) &&
          NS_SUCCEEDED(nsContentUtils::URIInheritsSecurityContext(aURI,
                                                                  &inherits)) &&
          inherits) {
       principalToInherit = GetInheritedPrincipal(true);
     }
   }
 
-  // Don't allow loads that would inherit our security context
-  // if this document came from an unsafe channel.
-  {
-    bool willInherit;
-    // This condition needs to match the one in
-    // nsContentUtils::ChannelShouldInheritPrincipal.
-    // Except we reverse the rv check to be safe in case
-    // nsContentUtils::URIInheritsSecurityContext fails here and
-    // succeeds there.
-    rv = nsContentUtils::URIInheritsSecurityContext(aURI, &willInherit);
-    if (NS_FAILED(rv) || willInherit || NS_IsAboutBlank(aURI)) {
-      nsCOMPtr<nsIDocShellTreeItem> treeItem = this;
-      do {
-        nsCOMPtr<nsIDocShell> itemDocShell = do_QueryInterface(treeItem);
-        bool isUnsafe;
-        if (itemDocShell &&
-            NS_SUCCEEDED(itemDocShell->GetChannelIsUnsafe(&isUnsafe)) &&
-            isUnsafe) {
-          return NS_ERROR_DOM_SECURITY_ERR;
-        }
-
-        nsCOMPtr<nsIDocShellTreeItem> parent;
-        treeItem->GetSameTypeParent(getter_AddRefs(parent));
-        parent.swap(treeItem);
-      } while (treeItem);
-    }
-  }
-
   nsIDocument* doc = mContentViewer ? mContentViewer->GetDocument()
                                     : nullptr;
 
   const bool isDocumentAuxSandboxed = doc &&
     (doc->GetSandboxFlags() & SANDBOXED_AUXILIARY_NAVIGATION);
 
   if (aURI && mLoadURIDelegate &&
       (!targetDocShell || targetDocShell == static_cast<nsIDocShell*>(this))) {
--- a/docshell/base/nsIDocShell.idl
+++ b/docshell/base/nsIDocShell.idl
@@ -578,23 +578,16 @@ interface nsIDocShell : nsIDocShellTreeI
 
   /**
    * Find out whether the docshell is currently in the middle of a page
    * transition. This is set just before the pagehide/unload events fire.
    */
   readonly attribute boolean isInUnload;
 
   /**
-   * Find out if the currently loaded document came from a suspicious channel
-   * (such as a JAR channel where the server-returned content type isn't a
-   * known JAR type).
-   */
-  readonly attribute boolean channelIsUnsafe;
-
-  /**
    * This attribute determines whether Mixed Active Content is loaded on the
    * document. When it is true, mixed active content was not blocked and has
    * loaded (or is about to load) on the page. When it is false, mixed active content
    * has not loaded on the page, either because there was no mixed active content
    * requests on the page or such requests were blocked by nsMixedContentBlocker.
    * This boolean is set to true in nsMixedContentBlocker if Mixed Active Content
    * is allowed (either explicitly on the page by the user or when the about:config
    * setting security.mixed_content.block_active_content is set to false).
--- a/dom/base/nsGlobalWindowOuter.cpp
+++ b/dom/base/nsGlobalWindowOuter.cpp
@@ -2000,24 +2000,16 @@ nsGlobalWindowOuter::SetNewDocument(nsID
       JS::Rooted<JSObject*> obj(cx, newInnerGlobal);
       rv = kungFuDeathGrip->InitClasses(obj);
       NS_ENSURE_SUCCESS(rv, rv);
 
       rv = newInnerWindow->ExecutionReady();
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
-    // If the document comes from a JAR, check if the channel was determined
-    // to be unsafe. If so, permanently disable script on the compartment by
-    // calling Block() and throwing away the key.
-    nsCOMPtr<nsIJARChannel> jarChannel = do_QueryInterface(aDocument->GetChannel());
-    if (jarChannel && jarChannel->GetIsUnsafe()) {
-      xpc::Scriptability::Get(newInnerGlobal).Block();
-    }
-
     if (mArguments) {
       newInnerWindow->DefineArgumentsProperty(mArguments);
       mArguments = nullptr;
     }
 
     // Give the new inner window our chrome event handler (since it
     // doesn't have one).
     newInnerWindow->mChromeEventHandler = mChromeEventHandler;
--- a/modules/libjar/nsIJARChannel.idl
+++ b/modules/libjar/nsIJARChannel.idl
@@ -7,24 +7,16 @@
 
 interface nsIFile;
 interface nsIZipEntry;
 
 [scriptable, builtinclass, uuid(e72b179b-d5df-4d87-b5de-fd73a65c60f6)]
 interface nsIJARChannel : nsIChannel
 {
     /**
-     * Returns TRUE if the JAR file is not safe (if the content type reported
-     * by the server for a remote JAR is not of an expected type).  Scripting,
-     * redirects, and plugins should be disabled when loading from this
-     * channel.
-     */
-    [infallible] readonly attribute boolean isUnsafe;
-
-    /**
      * Returns the JAR file.  May be null if the jar is remote.
      * Setting the JAR file is optional and overrides the JAR
      * file used for local file JARs. Setting the JAR file after
      * the channel has been opened is not permitted.
      */
     attribute nsIFile jarFile;
 
     /**
--- a/modules/libjar/nsJARChannel.cpp
+++ b/modules/libjar/nsJARChannel.cpp
@@ -195,17 +195,16 @@ nsJARInputThunk::IsNonBlocking(bool *non
 nsJARChannel::nsJARChannel()
     : mOpened(false)
     , mContentLength(-1)
     , mLoadFlags(LOAD_NORMAL)
     , mStatus(NS_OK)
     , mIsPending(false)
     , mEnableOMT(true)
     , mPendingEvent()
-    , mIsUnsafe(true)
 {
     LOG(("nsJARChannel::nsJARChannel [this=%p]\n", this));
     // hold an owning reference to the jar handler
     mJarHandler = gJarHandler;
 }
 
 nsJARChannel::~nsJARChannel()
 {
@@ -429,19 +428,16 @@ nsJARChannel::OpenLocalFile()
     LOG(("nsJARChannel::OpenLocalFile [this=%p]\n", this));
 
     MOZ_ASSERT(NS_IsMainThread());
 
     MOZ_ASSERT(mWorker);
     MOZ_ASSERT(mIsPending);
     MOZ_ASSERT(mJarFile);
 
-    // Local files are always considered safe.
-    mIsUnsafe = false;
-
     nsresult rv;
 
     // Set mLoadGroup and mOpened before AsyncOpen return, and set back if
     // if failed when callback.
     if (mLoadGroup) {
         mLoadGroup->AddRequest(this, nullptr);
     }
     mOpened = true;
@@ -935,17 +931,16 @@ NS_IMETHODIMP
 nsJARChannel::Open(nsIInputStream **stream)
 {
     LOG(("nsJARChannel::Open [this=%p]\n", this));
 
     NS_ENSURE_TRUE(!mOpened, NS_ERROR_IN_PROGRESS);
     NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
 
     mJarFile = nullptr;
-    mIsUnsafe = true;
 
     nsresult rv = LookupFile();
     if (NS_FAILED(rv))
         return rv;
 
     // If mJarFile was not set by LookupFile, we can't open a channel.
     if (!mJarFile) {
         NS_NOTREACHED("only file-backed jars are supported");
@@ -954,18 +949,16 @@ nsJARChannel::Open(nsIInputStream **stre
 
     RefPtr<nsJARInputThunk> input;
     rv = CreateJarInput(gJarHandler->JarCache(), getter_AddRefs(input));
     if (NS_FAILED(rv))
         return rv;
 
     input.forget(stream);
     mOpened = true;
-    // local files are always considered safe
-    mIsUnsafe = false;
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsJARChannel::Open2(nsIInputStream** aStream)
 {
     LOG(("nsJARChannel::Open2 [this=%p]\n", this));
     nsCOMPtr<nsIStreamListener> listener;
@@ -985,17 +978,16 @@ nsJARChannel::AsyncOpen(nsIStreamListene
                 nsContentUtils::IsSystemPrincipal(mLoadInfo->LoadingPrincipal())),
                "security flags in loadInfo but asyncOpen2() not called");
 
     NS_ENSURE_ARG_POINTER(listener);
     NS_ENSURE_TRUE(!mOpened, NS_ERROR_IN_PROGRESS);
     NS_ENSURE_TRUE(!mIsPending, NS_ERROR_IN_PROGRESS);
 
     mJarFile = nullptr;
-    mIsUnsafe = true;
 
     // Initialize mProgressSink
     NS_QueryNotificationCallbacks(mCallbacks, mLoadGroup, mProgressSink);
 
     mListener = listener;
     mListenerContext = ctx;
     mIsPending = true;
 
@@ -1040,23 +1032,16 @@ nsJARChannel::AsyncOpen2(nsIStreamListen
 
     return AsyncOpen(listener, nullptr);
 }
 
 //-----------------------------------------------------------------------------
 // nsIJARChannel
 //-----------------------------------------------------------------------------
 NS_IMETHODIMP
-nsJARChannel::GetIsUnsafe(bool *isUnsafe)
-{
-    *isUnsafe = mIsUnsafe;
-    return NS_OK;
-}
-
-NS_IMETHODIMP
 nsJARChannel::GetJarFile(nsIFile **aFile)
 {
     NS_IF_ADDREF(*aFile = mJarFile);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsJARChannel::SetJarFile(nsIFile *aFile)
--- a/modules/libjar/nsJARChannel.h
+++ b/modules/libjar/nsJARChannel.h
@@ -90,18 +90,16 @@ private:
 
     bool                            mEnableOMT;
     // |Cancel()|, |Suspend()|, and |Resume()| might be called during AsyncOpen.
     struct {
         bool isCanceled;
         uint32_t suspendCount;
     }                               mPendingEvent;
 
-    bool                            mIsUnsafe;
-
     nsCOMPtr<nsIInputStreamPump>    mPump;
     // mRequest is only non-null during OnStartRequest, so we'll have a pointer
     // to the request if we get called back via RetargetDeliveryTo.
     nsCOMPtr<nsIRequest>            mRequest;
     nsCOMPtr<nsIFile>               mJarFile;
     nsCOMPtr<nsIFile>               mJarFileOverride;
     nsCOMPtr<nsIZipReader>          mPreCachedJarReader;
     nsCOMPtr<nsIURI>                mJarBaseURI;