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
--- a/media/mtransport/transportlayerice.cpp
+++ b/media/mtransport/transportlayerice.cpp
@@ -94,16 +94,27 @@ TransportLayerIce::TransportLayerIce(con
TransportLayerIce::~TransportLayerIce() {
// No need to do anything here, since we use smart pointers
}
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.
if (stream_ && !old_stream_ && (stream_ != stream)) {
// Here we leave the old stream's signals connected until we don't need
// it anymore. They will be disconnected if ice restart is successful.
old_stream_ = stream_;
MOZ_MTLOG(ML_INFO, LAYER_INFO << "SetParameters save old stream("
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.cpp
@@ -722,16 +722,17 @@ JsepSessionImpl::GetRemoteIds(const Sdp&
return rv;
}
nsresult
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));
return NS_ERROR_UNEXPECTED;
}
// Undo track assignments from a previous call to CreateOffer
// (ie; if the track has not been negotiated yet, it doesn't necessarily need
@@ -1989,16 +1990,25 @@ JsepSessionImpl::ValidateRemoteDescripti
if (oldMsection.GetMediaType() != newMsection.GetMediaType()) {
JSEP_SET_ERROR("Remote description changes the media type of m-line "
<< i);
return NS_ERROR_INVALID_ARG;
}
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
iceCredsDiffer = mozilla::Some(differ);
} else if (iceCredsDiffer.isSome() && *iceCredsDiffer != differ) {
// subsequent msections must match the first sections
JSEP_SET_ERROR("Partial ICE restart is unsupported at this time "
"(new remote description changes either the ice-ufrag "
--- a/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
+++ b/media/webrtc/signaling/src/jsep/JsepSessionImpl.h
@@ -29,16 +29,17 @@ public:
class JsepSessionImpl : public JsepSession
{
public:
JsepSessionImpl(const std::string& name, UniquePtr<JsepUuidGenerator> uuidgen)
: JsepSession(name),
mIsOfferer(false),
mWasOffererLastTime(false),
mIceControlling(false),
+ mLocalIceIsRestarting(false),
mRemoteIsIceLite(false),
mRemoteIceIsRestarting(false),
mBundlePolicy(kBundleBalanced),
mSessionId(0),
mSessionVersion(0),
mUuidGen(Move(uuidgen)),
mSdpHelper(&mLastError)
{
@@ -317,16 +318,17 @@ private:
std::vector<RefPtr<JsepTransport> > mOldTransports;
std::vector<JsepTrackPair> mNegotiatedTrackPairs;
bool mIsOfferer;
bool mWasOffererLastTime;
bool mIceControlling;
std::string mIceUfrag;
std::string mIcePwd;
+ bool mLocalIceIsRestarting;
bool mRemoteIsIceLite;
bool mRemoteIceIsRestarting;
std::vector<std::string> mIceOptions;
JsepBundlePolicy mBundlePolicy;
std::vector<JsepDtlsFingerprint> mDtlsFingerprints;
uint64_t mSessionId;
uint64_t mSessionVersion;
std::vector<SdpExtmapAttributeList::Extmap> mAudioRtpExtensions;