Verified Commit e7de1788 authored by Eden Chuang's avatar Eden Chuang Committed by ma1
Browse files

Bug 2039452 - Validate file:// URIs in PExternalHelperApp against the sending... 

Bug 2039452 - Validate file:// URIs in PExternalHelperApp against the sending process's remote type.  a=RyanVM DONTBUILD

The parent derived the native helper-launch decision from a
content-process supplied aWasFileChannel flag and URI rather than
binding it to the sending process's capabilities.

Stop sending aWasFileChannel over IPC and derive it on the parent from
the actual URI, and reject a file:// URI in PExternalHelperApp from a
process that is not allowed to load local files (mirrors the file://
policy in ValidatePrincipalCouldPotentiallyBeLoadedBy).

Original Revision: https://phabricator.services.mozilla.com/D309577

Differential Revision: https://phabricator.services.mozilla.com/D311929
parent a8580110

dom/ipc/ContentParent.cpp

+19 −6
Original line number Diff line number Diff line
@@ -73,6 +73,7 @@ 
#include "mozilla/ScriptPreloader.h"

#include "mozilla/Components.h"

#include "mozilla/Sprintf.h"

#include "mozilla/StaticPrefs_browser.h"

#include "mozilla/StaticPrefs_dom.h"

#include "mozilla/StaticPrefs_fission.h"

#include "mozilla/StaticPrefs_media.h"
@@ -4547,11 +4548,11 @@ ContentParent::AllocPExternalHelperAppParent( 
    const nsACString& aMimeContentType, const nsACString& aContentDisposition,

    const uint32_t& aContentDispositionHint,

    const nsAString& aContentDispositionFilename, const bool& aForceSave,

    const int64_t& aContentLength, const bool& aWasFileChannel,

    nsIURI* aReferrer, const MaybeDiscarded<BrowsingContext>& aContext) {

    const int64_t& aContentLength, nsIURI* aReferrer,

    const MaybeDiscarded<BrowsingContext>& aContext) {

  RefPtr<ExternalHelperAppParent> parent = new ExternalHelperAppParent(

      uri, aContentLength, aWasFileChannel, aContentDisposition,

      aContentDispositionHint, aContentDispositionFilename);

      uri, aContentLength, aContentDisposition, aContentDispositionHint,

      aContentDispositionFilename);

  return parent.forget();

}
@@ -4561,8 +4562,20 @@ mozilla::ipc::IPCResult ContentParent::RecvPExternalHelperAppConstructor( 
    const nsACString& aContentDisposition,

    const uint32_t& aContentDispositionHint,

    const nsAString& aContentDispositionFilename, const bool& aForceSave,

    const int64_t& aContentLength, const bool& aWasFileChannel,

    nsIURI* aReferrer, const MaybeDiscarded<BrowsingContext>& aContext) {

    const int64_t& aContentLength, nsIURI* aReferrer,

    const MaybeDiscarded<BrowsingContext>& aContext) {

  // A content process must never be able to drive a native helper-app launch

  // of a local file it chose: that decision has to be bound to a channel the

  // process was actually allowed to open. A file:// URI can only be loaded in

  // a file content process (mirrors ValidatePrincipalCouldPotentiallyBeLoadedBy

  // and the sandboxing policy enforced in ProcessIsolation), so reject a

  // file:// URI coming from any other process.

  if (uri && uri->SchemeIs("file") &&

      StaticPrefs::browser_tabs_remote_separateFileUriProcess() &&

      GetRemoteType() != FILE_REMOTE_TYPE) {

    return IPC_FAIL(this, "Non-file process sent a file:// URI.");

  }



  BrowsingContext* context = aContext.IsDiscarded() ? nullptr : aContext.get();

  if (!static_cast<ExternalHelperAppParent*>(actor)->Init(

          loadInfoArgs, aMimeContentType, aForceSave, aReferrer, context)) {

dom/ipc/ContentParent.h

+3 −4
Original line number Diff line number Diff line
@@ -942,8 +942,8 @@ class ContentParent final : public PContentParent, 
      const nsACString& aMimeContentType, const nsACString& aContentDisposition,

      const uint32_t& aContentDispositionHint,

      const nsAString& aContentDispositionFilename, const bool& aForceSave,

      const int64_t& aContentLength, const bool& aWasFileChannel,

      nsIURI* aReferrer, const MaybeDiscarded<BrowsingContext>& aContext);

      const int64_t& aContentLength, nsIURI* aReferrer,

      const MaybeDiscarded<BrowsingContext>& aContext);



  mozilla::ipc::IPCResult RecvPExternalHelperAppConstructor(

      PExternalHelperAppParent* actor, nsIURI* uri,
@@ -951,8 +951,7 @@ class ContentParent final : public PContentParent, 
      const nsACString& aContentDisposition,

      const uint32_t& aContentDispositionHint,

      const nsAString& aContentDispositionFilename, const bool& aForceSave,

      const int64_t& aContentLength, const bool& aWasFileChannel,

      nsIURI* aReferrer,

      const int64_t& aContentLength, nsIURI* aReferrer,

      const MaybeDiscarded<BrowsingContext>& aContext) override;



  already_AddRefed<PHandlerServiceParent> AllocPHandlerServiceParent();

dom/ipc/PContent.ipdl

+0 −1
Original line number Diff line number Diff line
@@ -1196,7 +1196,6 @@ parent: 
                             nsString aContentDispositionFilename,

                             bool aForceSave,

                             int64_t aContentLength,

                             bool aWasFileChannel,

                             nullable nsIURI aReferrer,

                             MaybeDiscardedBrowsingContext aContext);

uriloader/exthandler/ExternalHelperAppParent.cpp

+4 −2
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ NS_IMPL_ISUPPORTS_INHERITED(ExternalHelperAppParent, nsHashPropertyBag, 
                            nsIStreamListener, nsIExternalHelperAppParent)



ExternalHelperAppParent::ExternalHelperAppParent(

    nsIURI* uri, const int64_t& aContentLength, const bool& aWasFileChannel,

    nsIURI* uri, const int64_t& aContentLength,

    const nsACString& aContentDispositionHeader,

    const uint32_t& aContentDispositionHint,

    const nsAString& aContentDispositionFilename)
@@ -46,7 +46,9 @@ ExternalHelperAppParent::ExternalHelperAppParent( 
      mStatus(NS_OK),

      mCanceled(false),

      mContentLength(aContentLength),

      mWasFileChannel(aWasFileChannel) {

      // Never trust a child-supplied flag for the native helper-launch

      // decision: derive it from the actual URI the parent will operate on.

      mWasFileChannel(uri && uri->SchemeIs("file")) {

  mContentDispositionHeader = aContentDispositionHeader;

  if (!mContentDispositionHeader.IsEmpty()) {

    NS_GetFilenameFromDisposition(mContentDispositionFilename,

uriloader/exthandler/ExternalHelperAppParent.h

+0 −1
Original line number Diff line number Diff line
@@ -74,7 +74,6 @@ class ExternalHelperAppParent 
  bool WasFileChannel() override { return mWasFileChannel; }



  ExternalHelperAppParent(nsIURI* uri, const int64_t& contentLength,

                          const bool& wasFileChannel,

                          const nsACString& aContentDispositionHeader,

                          const uint32_t& aContentDispositionHint,

                          const nsAString& aContentDispositionFilename);