summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorliberato@chromium.org <liberato@chromium.org>2019-10-15 19:09:42 +0000
committerMichal Klocek <michal.klocek@qt.io>2020-01-16 13:22:12 +0000
commit41d474d0e7c67023ea4da5a1c8e105b7cc641df1 (patch)
tree0d823e3964a1907f97b1c1022f184f69e821efcf
parente687bc69cdc906439cfb257aed3af3eae5d92f52 (diff)
downloadqtwebengine-chromium-41d474d0e7c67023ea4da5a1c8e105b7cc641df1.tar.gz
[Backport] CVE-2019-13745 1/2
Capture redirect target for media download. Instead of using the current src URL when downloading media, use the final target after all redirects from resource selection. Additionally, do not download the media file if any cross-origin redirect is requested by the server. Bug: 990867 Change-Id: Ic7c708c4001bae81aa85a158aed9e109c3297c93 Reviewed-by: Michael BrĂ¼ning <michael.bruning@qt.io>
-rw-r--r--chromium/media/blink/webmediaplayer_impl.cc4
-rw-r--r--chromium/media/blink/webmediaplayer_impl.h1
-rw-r--r--chromium/third_party/blink/public/platform/web_media_player.h4
-rw-r--r--chromium/third_party/blink/renderer/core/html/media/html_media_element.cc20
-rw-r--r--chromium/third_party/blink/renderer/core/html/media/html_media_element.h12
-rw-r--r--chromium/third_party/blink/renderer/core/html/media/html_media_element_test.cc30
-rw-r--r--chromium/third_party/blink/renderer/modules/media_controls/elements/media_control_download_button_element.cc2
7 files changed, 69 insertions, 4 deletions
diff --git a/chromium/media/blink/webmediaplayer_impl.cc b/chromium/media/blink/webmediaplayer_impl.cc
index 0fe502b37df..5d6dcd8dceb 100644
--- a/chromium/media/blink/webmediaplayer_impl.cc
+++ b/chromium/media/blink/webmediaplayer_impl.cc
@@ -3594,4 +3594,8 @@ void WebMediaPlayerImpl::MaybeUpdateBufferSizesForPlayback() {
UpdateMediaPositionState();
}
+GURL WebMediaPlayerImpl::GetSrcAfterRedirects() {
+ return mb_data_source_ ? mb_data_source_->GetUrlAfterRedirects() : GURL();
+}
+
} // namespace media
diff --git a/chromium/media/blink/webmediaplayer_impl.h b/chromium/media/blink/webmediaplayer_impl.h
index ec2b9694dd3..612146c4a8e 100644
--- a/chromium/media/blink/webmediaplayer_impl.h
+++ b/chromium/media/blink/webmediaplayer_impl.h
@@ -282,6 +282,7 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
bool IsOpaque() const override;
int GetDelegateId() override;
base::Optional<viz::SurfaceId> GetSurfaceId() override;
+ GURL GetSrcAfterRedirects() override;
base::WeakPtr<blink::WebMediaPlayer> AsWeakPtr() override;
diff --git a/chromium/third_party/blink/public/platform/web_media_player.h b/chromium/third_party/blink/public/platform/web_media_player.h
index 61869a8bf55..c2c3e215e57 100644
--- a/chromium/third_party/blink/public/platform/web_media_player.h
+++ b/chromium/third_party/blink/public/platform/web_media_player.h
@@ -425,6 +425,10 @@ class WebMediaPlayer {
return base::nullopt;
}
+ // Provide the media URL, after any redirects are applied. May return an
+ // empty GURL, which will be interpreted as "use the original URL".
+ virtual GURL GetSrcAfterRedirects() { return GURL(); }
+
virtual base::WeakPtr<WebMediaPlayer> AsWeakPtr() = 0;
};
diff --git a/chromium/third_party/blink/renderer/core/html/media/html_media_element.cc b/chromium/third_party/blink/renderer/core/html/media/html_media_element.cc
index 637245eb50f..0506fbbca58 100644
--- a/chromium/third_party/blink/renderer/core/html/media/html_media_element.cc
+++ b/chromium/third_party/blink/renderer/core/html/media/html_media_element.cc
@@ -1165,6 +1165,10 @@ void HTMLMediaElement::LoadResource(const WebMediaPlayerSource& source,
// media element API.
current_src_ = url;
+ // Default this to empty, so that we use |current_src_| unless the player
+ // provides one later.
+ current_src_after_redirects_ = KURL();
+
if (audio_source_node_)
audio_source_node_->OnCurrentSrcChanged(current_src_);
@@ -1775,6 +1779,13 @@ void HTMLMediaElement::SetReadyState(ReadyState state) {
ready_state_ = kHaveCurrentData;
}
+ // If we're transitioning to / past kHaveMetadata, then cache the final URL.
+ if (old_state < kHaveMetadata && new_state >= kHaveMetadata &&
+ web_media_player_) {
+ current_src_after_redirects_ =
+ KURL(web_media_player_->GetSrcAfterRedirects());
+ }
+
if (new_state > ready_state_maximum_)
ready_state_maximum_ = new_state;
@@ -1951,8 +1962,11 @@ bool HTMLMediaElement::SupportsSave() const {
return false;
}
+ // Get the URL that we'll use for downloading.
+ const KURL url = downloadURL();
+
// URLs that lead to nowhere are ignored.
- if (current_src_.IsNull() || current_src_.IsEmpty())
+ if (url.IsNull() || url.IsEmpty())
return false;
// If we have no source, we can't download.
@@ -1960,7 +1974,7 @@ bool HTMLMediaElement::SupportsSave() const {
return false;
// It is not useful to offer a save feature on local files.
- if (current_src_.IsLocalFile())
+ if (url.IsLocalFile())
return false;
// MediaStream can't be downloaded.
@@ -1972,7 +1986,7 @@ bool HTMLMediaElement::SupportsSave() const {
return false;
// HLS stream shouldn't have a download button.
- if (IsHLSURL(current_src_))
+ if (IsHLSURL(url))
return false;
// Infinite streams don't have a clear end at which to finish the download.
diff --git a/chromium/third_party/blink/renderer/core/html/media/html_media_element.h b/chromium/third_party/blink/renderer/core/html/media/html_media_element.h
index ea0853391dd..1584d203b97 100644
--- a/chromium/third_party/blink/renderer/core/html/media/html_media_element.h
+++ b/chromium/third_party/blink/renderer/core/html/media/html_media_element.h
@@ -143,6 +143,17 @@ class CORE_EXPORT HTMLMediaElement
void SetSrc(const AtomicString&);
void SetSrc(const USVStringOrTrustedURL&, ExceptionState&);
const KURL& currentSrc() const { return current_src_; }
+
+ // Return the URL to be used for downloading the media.
+ const KURL& downloadURL() const {
+ // If we didn't get a redirected URL from the player, then use the original.
+ if (current_src_after_redirects_.IsNull() ||
+ current_src_after_redirects_.IsEmpty()) {
+ return currentSrc();
+ }
+ return current_src_after_redirects_;
+ }
+
void SetSrcObject(MediaStreamDescriptor*);
MediaStreamDescriptor* GetSrcObject() const { return src_object_.Get(); }
@@ -572,6 +583,7 @@ class CORE_EXPORT HTMLMediaElement
ReadyState ready_state_;
ReadyState ready_state_maximum_;
KURL current_src_;
+ KURL current_src_after_redirects_;
Member<MediaStreamDescriptor> src_object_;
Member<MediaError> error_;
diff --git a/chromium/third_party/blink/renderer/core/html/media/html_media_element_test.cc b/chromium/third_party/blink/renderer/core/html/media/html_media_element_test.cc
index 5e161688368..abcad2e49a3 100644
--- a/chromium/third_party/blink/renderer/core/html/media/html_media_element_test.cc
+++ b/chromium/third_party/blink/renderer/core/html/media/html_media_element_test.cc
@@ -45,6 +45,8 @@ class MockWebMediaPlayer : public EmptyWebMediaPlayer {
const blink::WebMediaPlayerSource& source,
CorsMode cors_mode));
MOCK_CONST_METHOD0(DidLazyLoad, bool());
+
+ MOCK_METHOD0(GetSrcAfterRedirects, GURL());
};
class WebMediaStubLocalFrameClient : public EmptyLocalFrameClient {
@@ -520,4 +522,32 @@ TEST_P(HTMLMediaElementTest, GcMarkingNoAllocHasActivity) {
Media()->HasPendingActivity();
}
+TEST_P(HTMLMediaElementTest, CapturesRedirectedSrc) {
+ // Verify that the element captures the redirected URL.
+ Media()->SetSrc(SrcSchemeToURL(TestURLScheme::kHttp));
+ Media()->Play();
+ test::RunPendingTasks();
+
+ // Should start at the original.
+ EXPECT_EQ(Media()->downloadURL(), Media()->currentSrc());
+
+ GURL redirected_url("https://redirected.com");
+ EXPECT_CALL(*MockMediaPlayer(), GetSrcAfterRedirects())
+ .WillRepeatedly(Return(redirected_url));
+ SetReadyState(HTMLMediaElement::kHaveFutureData);
+
+ EXPECT_EQ(Media()->downloadURL(), redirected_url);
+}
+
+TEST_P(HTMLMediaElementTest, EmptyRedirectedSrcUsesOriginal) {
+ // If the player returns an empty URL for the redirected src, then the element
+ // should continue using currentSrc().
+ Media()->SetSrc(SrcSchemeToURL(TestURLScheme::kHttp));
+ Media()->Play();
+ test::RunPendingTasks();
+ EXPECT_EQ(Media()->downloadURL(), Media()->currentSrc());
+ SetReadyState(HTMLMediaElement::kHaveFutureData);
+ EXPECT_EQ(Media()->downloadURL(), Media()->currentSrc());
+}
+
} // namespace blink
diff --git a/chromium/third_party/blink/renderer/modules/media_controls/elements/media_control_download_button_element.cc b/chromium/third_party/blink/renderer/modules/media_controls/elements/media_control_download_button_element.cc
index 5f8267cbda9..2e810d88f5e 100644
--- a/chromium/third_party/blink/renderer/modules/media_controls/elements/media_control_download_button_element.cc
+++ b/chromium/third_party/blink/renderer/modules/media_controls/elements/media_control_download_button_element.cc
@@ -68,7 +68,7 @@ const char* MediaControlDownloadButtonElement::GetNameForHistograms() const {
}
void MediaControlDownloadButtonElement::DefaultEventHandler(Event& event) {
- const KURL& url = MediaElement().currentSrc();
+ const KURL& url = MediaElement().downloadURL();
if ((event.type() == event_type_names::kClick ||
event.type() == event_type_names::kGesturetap) &&
!(url.IsNull() || url.IsEmpty())) {