From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Tony Herre Date: Fri, 1 Oct 2021 19:18:45 +0000 Subject: Check direction of RTCEncodedFrames Add a check to RTCEncodedVideoUnderlyingSink of the direction of the underlying webrtc frame, to make sure a web app doesn't take a received encoded frame and pass it into a sender insertable stream, which is as yet unsupported in WebRTC. Bug: 1247260 Change-Id: I9ed5bd8b2bd5e5ee461f3b553f8a91f6cc2e9ed7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3190473 Commit-Queue: Tony Herre Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#927323} diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.cc b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.cc index c390ab72418194cb10c3b0bc5a83b95de8dd19f6..775b837fee46836fd292b17ac8d80e4c83bd08a8 100644 --- a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.cc +++ b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.cc @@ -14,8 +14,10 @@ namespace blink { RTCEncodedVideoUnderlyingSink::RTCEncodedVideoUnderlyingSink( ScriptState* script_state, - TransformerCallback transformer_callback) - : transformer_callback_(std::move(transformer_callback)) { + TransformerCallback transformer_callback, + webrtc::TransformableFrameInterface::Direction expected_direction) + : transformer_callback_(std::move(transformer_callback)), + expected_direction_(expected_direction) { DCHECK(transformer_callback_); } @@ -53,6 +55,12 @@ ScriptPromise RTCEncodedVideoUnderlyingSink::write( return ScriptPromise(); } + if (webrtc_frame->GetDirection() != expected_direction_) { + exception_state.ThrowDOMException(DOMExceptionCode::kOperationError, + "Invalid frame"); + return ScriptPromise(); + } + RTCEncodedVideoStreamTransformer* transformer = transformer_callback_.Run(); if (!transformer) { exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError, diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.h b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.h index dd1cad227eb7947dd0bf2ec7ba217956cb7a8787..8591fcc6eb1c78d0e107e4f097d3133d111ab959 100644 --- a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.h +++ b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink.h @@ -7,6 +7,7 @@ #include "third_party/blink/renderer/core/streams/underlying_sink_base.h" #include "third_party/blink/renderer/modules/modules_export.h" +#include "third_party/webrtc/api/frame_transformer_interface.h" namespace blink { @@ -18,7 +19,9 @@ class MODULES_EXPORT RTCEncodedVideoUnderlyingSink final public: using TransformerCallback = base::RepeatingCallback; - RTCEncodedVideoUnderlyingSink(ScriptState*, TransformerCallback); + RTCEncodedVideoUnderlyingSink(ScriptState*, + TransformerCallback, + webrtc::TransformableFrameInterface::Direction); // UnderlyingSinkBase ScriptPromise start(ScriptState*, @@ -37,6 +40,7 @@ class MODULES_EXPORT RTCEncodedVideoUnderlyingSink final private: TransformerCallback transformer_callback_; + webrtc::TransformableFrameInterface::Direction expected_direction_; }; } // namespace blink diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink_test.cc b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink_test.cc index 614d16e9a43e493c35b82b838f46c4d7c01b83ac..465be4cd1406fdbfe777e68b538ab59bef16a0cd 100644 --- a/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink_test.cc +++ b/third_party/blink/renderer/modules/peerconnection/rtc_encoded_video_underlying_sink_test.cc @@ -74,11 +74,15 @@ class RTCEncodedVideoUnderlyingSinkTest : public testing::Test { EXPECT_FALSE(transformer_.HasTransformedFrameSinkCallback(kSSRC)); } - RTCEncodedVideoUnderlyingSink* CreateSink(ScriptState* script_state) { + RTCEncodedVideoUnderlyingSink* CreateSink( + ScriptState* script_state, + webrtc::TransformableFrameInterface::Direction expected_direction = + webrtc::TransformableFrameInterface::Direction::kSender) { return MakeGarbageCollected( script_state, WTF::BindRepeating(&RTCEncodedVideoUnderlyingSinkTest::GetTransformer, - WTF::Unretained(this))); + WTF::Unretained(this)), + expected_direction); } RTCEncodedVideoUnderlyingSink* CreateNullCallbackSink( @@ -86,15 +90,21 @@ class RTCEncodedVideoUnderlyingSinkTest : public testing::Test { return MakeGarbageCollected( script_state, WTF::BindRepeating( - []() -> RTCEncodedVideoStreamTransformer* { return nullptr; })); + []() -> RTCEncodedVideoStreamTransformer* { return nullptr; }), + webrtc::TransformableFrameInterface::Direction::kSender); } RTCEncodedVideoStreamTransformer* GetTransformer() { return &transformer_; } - ScriptValue CreateEncodedVideoFrameChunk(ScriptState* script_state) { + ScriptValue CreateEncodedVideoFrameChunk( + ScriptState* script_state, + webrtc::TransformableFrameInterface::Direction direction = + webrtc::TransformableFrameInterface::Direction::kSender) { auto mock_frame = std::make_unique>(); + ON_CALL(*mock_frame.get(), GetSsrc).WillByDefault(Return(kSSRC)); + ON_CALL(*mock_frame.get(), GetDirection).WillByDefault(Return(direction)); RTCEncodedVideoFrame* frame = MakeGarbageCollected(std::move(mock_frame)); return ScriptValue(script_state->GetIsolate(), @@ -175,4 +185,21 @@ TEST_F(RTCEncodedVideoUnderlyingSinkTest, WriteToNullCallbackSinkFails) { DOMExceptionCode::kInvalidStateError)); } +TEST_F(RTCEncodedVideoUnderlyingSinkTest, WriteInvalidDirectionFails) { + V8TestingScope v8_scope; + ScriptState* script_state = v8_scope.GetScriptState(); + auto* sink = CreateSink( + script_state, webrtc::TransformableFrameInterface::Direction::kSender); + + // Write an encoded chunk with direction set to Receiver should fail as it + // doesn't match the expected direction of our sink. + DummyExceptionStateForTesting dummy_exception_state; + sink->write(script_state, + CreateEncodedVideoFrameChunk( + script_state, + webrtc::TransformableFrameInterface::Direction::kReceiver), + nullptr, dummy_exception_state); + EXPECT_TRUE(dummy_exception_state.HadException()); +} + } // namespace blink diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc index 173e1965cbc18335c267731ec84e48af6f7c8c9b..72ee6eddfbefc7d745f4ba4b623a3ff1f9fa2baf 100644 --- a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc +++ b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_receiver.cc @@ -507,7 +507,8 @@ void RTCRtpReceiver::InitializeEncodedVideoStreams(ScriptState* script_state) { ->GetEncodedVideoStreamTransformer() : nullptr; }, - WrapWeakPersistent(this))); + WrapWeakPersistent(this)), + webrtc::TransformableFrameInterface::Direction::kReceiver); // The high water mark for the stream is set to 1 so that the stream seems // ready to write, but without queuing frames. WritableStream* writable_stream = diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc index c9a9b8f7692cfa37e591789a43169a58c69a0658..c54b671bfda084bc3d1f56b342a344298877435d 100644 --- a/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc +++ b/third_party/blink/renderer/modules/peerconnection/rtc_rtp_sender.cc @@ -907,7 +907,8 @@ void RTCRtpSender::InitializeEncodedVideoStreams(ScriptState* script_state) { ->GetEncodedVideoStreamTransformer() : nullptr; }, - WrapWeakPersistent(this))); + WrapWeakPersistent(this)), + webrtc::TransformableFrameInterface::Direction::kSender); // The high water mark for the stream is set to 1 so that the stream is // ready to write, but without queuing frames. WritableStream* writable_stream =