From 90e456499316cafb63a877c6db400fd718dba603 Mon Sep 17 00:00:00 2001
From: Michael Froman <mfroman@mozilla.com>
Date: Thu, 12 Oct 2017 22:38:01 -0500
Subject: [PATCH] Bug 1405940 - Fix Null Pointer dereference in
 sigslot::lock_block r=bwc

Caused by several issues:
1) We were allowing an answer with modified ufrag/pass to
   begin an ICE restart even if the offer didn't indicate
   it was restarting.
2) This should no longer happen, but in cases where restart logic
   was started inappropriately, TransportLayerIce::SetParameters
   could get a null stream, and we check for that now.

MozReview-Commit-ID: JFQ1zz3l5wY

--HG--
extra : rebase_source : a6d43aabada86669850ddce07ea86da8118a6bec
---
 media/mtransport/transportlayerice.cpp              | 11 +++++++++++
 media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp | 10 ++++++++++
 media/webrtc/signaling/src/jsep/JsepSessionImpl.h   |  2 ++
 3 files changed, 23 insertions(+)

diff --git a/media/mtransport/transportlayerice.cpp b/media/mtransport/transportlayerice.cpp
index dba2c5424a8c..df4456bdd08a 100644
--- a/media/mtransport/transportlayerice.cpp
+++ b/media/mtransport/transportlayerice.cpp
@@ -99,6 +99,17 @@ TransportLayerIce::~TransportLayerIce() {
 void TransportLayerIce::SetParameters(RefPtr<NrIceCtx> ctx,
                                       RefPtr<NrIceMediaStream> stream,
                                       int component) {
+  // Stream could be null in the case of some badly written js that causes
+  // us to be in an ICE restart case, but not have valid streams due to
+  // not calling PeerConnectionMedia::EnsureTransports if
+  // PeerConnectionImpl::SetSignalingState_m thinks the conditions were
+  // not correct.  We also solved a case where an incoming answer was
+  // incorrectly beginning an ICE restart when the offer did not indicate one.
+  if (!stream) {
+    MOZ_ASSERT(false);
+    return;
+  }
+
   // If SetParameters is called and we already have a stream_, this means
   // we're handling an ICE restart.  We need to hold the old stream until
   // we know the new stream is working.
diff --git a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
index 8d8e6bc47553..8fcc24f0fdc9 100644
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -727,6 +727,7 @@ JsepSessionImpl::CreateOffer(const JsepOfferOptions& options,
                              std::string* offer)
 {
   mLastError.clear();
+  mLocalIceIsRestarting = options.mIceRestart.isSome() && *(options.mIceRestart);
 
   if (mState != kJsepStateStable) {
     JSEP_SET_ERROR("Cannot create offer in state " << GetStateStr(mState));
@@ -1994,6 +1995,15 @@ JsepSessionImpl::ValidateRemoteDescription(const Sdp& description)
     }
 
     bool differ = mSdpHelper.IceCredentialsDiffer(newMsection, oldMsection);
+
+    // Detect bad answer ICE restart when offer doesn't request ICE restart
+    if (mIsOfferer && differ && !mLocalIceIsRestarting) {
+      JSEP_SET_ERROR("Remote description indicates ICE restart but offer did not "
+                     "request ICE restart (new remote description changes either "
+                     "the ice-ufrag or ice-pwd)");
+      return NS_ERROR_INVALID_ARG;
+    }
+
     // Detect whether all the creds are the same or all are different
     if (!iceCredsDiffer.isSome()) {
       // for the first msection capture whether creds are different or same
diff --git a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
index 7c8b64919203..f398d2b7caa8 100644
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
@@ -34,6 +34,7 @@ public:
         mIsOfferer(false),
         mWasOffererLastTime(false),
         mIceControlling(false),
+        mLocalIceIsRestarting(false),
         mRemoteIsIceLite(false),
         mRemoteIceIsRestarting(false),
         mBundlePolicy(kBundleBalanced),
@@ -322,6 +323,7 @@ private:
   bool mIceControlling;
   std::string mIceUfrag;
   std::string mIcePwd;
+  bool mLocalIceIsRestarting;
   bool mRemoteIsIceLite;
   bool mRemoteIceIsRestarting;
   std::vector<std::string> mIceOptions;