summaryrefslogtreecommitdiffstats
diff options
authorDaniel Cheng <dcheng@chromium.org>2025-03-28 13:30:27 -0700
committerMichael Brüning <michael.bruning@qt.io>2025-05-02 15:16:55 +0000
commit35b2fd5a66733b0d57b94bdb1df2baad4d888f5e (patch)
treedb3f0407b0949a46f23cac8fcf9a6aec8e0037c2
parent37f56f75e9713727f79e44b4688febcea06f5e90 (diff)
[Backport] CVE-2025-4051: Insufficient data validation in DevTools122-based
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/6400553: Don't allow text -> URL conversion when dropping to bypass URL filtering When starting a drag from a renderer, the browser process filters out URLs that the initiating renderer process should not be able to navigate to, e.g. a random http/https page should not be able to specify chrome://settings/ as URL to navigate to when dropped. However, when dropping, Chrome is clever and tries to interpret text as URLs when needed. To prevent this from bypassing the URL filtering, only allow this conversion if: - the drag data does not originate from the renderer - or the text to URL conversion results in a HTTP or HTTPS url Bug: 404000989 Change-Id: I28baf7e6385b440af7e76b08471588299e24e247 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6400553 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/main@{#1439671} Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/643276 Reviewed-by: Anu Aliyas <anu.aliyas@qt.io>
Diffstat
-rw-r--r--chromium/ui/base/clipboard/clipboard_util_mac.mm14
-rw-r--r--chromium/ui/base/dragdrop/os_exchange_data_provider.h13
-rw-r--r--chromium/ui/base/dragdrop/os_exchange_data_provider_non_backed.cc14
-rw-r--r--chromium/ui/base/dragdrop/os_exchange_data_provider_win.cc7
-rw-r--r--chromium/ui/base/ui_base_features.cc4
-rw-r--r--chromium/ui/base/ui_base_features.h3
6 files changed, 49 insertions, 6 deletions
diff --git a/chromium/ui/base/clipboard/clipboard_util_mac.mm b/chromium/ui/base/clipboard/clipboard_util_mac.mm
index bf87f5a2789..263795b09d1 100644
--- a/chromium/ui/base/clipboard/clipboard_util_mac.mm
+++ b/chromium/ui/base/clipboard/clipboard_util_mac.mm
@@ -18,6 +18,7 @@
#include "ui/base/clipboard/clipboard_constants.h"
#include "ui/base/clipboard/file_info.h"
#include "ui/base/clipboard/url_file_parser.h"
+#include "ui/base/ui_base_features.h"
#include "url/gurl.h"
@interface URLAndTitle ()
@@ -267,7 +268,8 @@ URLAndTitle* ExtractURLFromURLFile(NSPasteboardItem* item) {
// Returns a URL and title if a string on the pasteboard item is formatted as a
// URL but doesn't actually have the URL type.
-URLAndTitle* ExtractURLFromStringValue(NSPasteboardItem* item) {
+URLAndTitle* ExtractURLFromStringValue(NSPasteboardItem* item,
+ bool is_renderer_tainted) {
NSString* string = [item stringForType:NSPasteboardTypeString];
if (!string) {
return nil;
@@ -289,6 +291,12 @@ URLAndTitle* ExtractURLFromStringValue(NSPasteboardItem* item) {
return nil;
}
+ if (base::FeatureList::IsEnabled(
+ features::kDragDropOnlySynthesizeHttpOrHttpsUrlsFromText) &&
+ is_renderer_tainted && !url.SchemeIsHTTPOrHTTPS()) {
+ return nil;
+ }
+
// The hostname is the best that can be done for the title.
return [URLAndTitle URLAndTitleWithURL:string
title:base::SysUTF8ToNSString(url.host())];
@@ -318,6 +326,8 @@ NSArray<URLAndTitle*>* ReadURLItemsWithTitles(NSPasteboard* pboard,
bool include_files) {
NSMutableArray<URLAndTitle*>* result = [NSMutableArray array];
+ const bool is_renderer_tainted =
+ [pboard.types containsObject:kUTTypeChromiumRendererInitiatedDrag];
for (NSPasteboardItem* item in pboard.pasteboardItems) {
// Try each of several ways of getting URLs from the pasteboard item and
// stop with the first one that works.
@@ -329,7 +339,7 @@ NSArray<URLAndTitle*>* ReadURLItemsWithTitles(NSPasteboard* pboard,
}
if (!url_and_title) {
- url_and_title = ExtractURLFromStringValue(item);
+ url_and_title = ExtractURLFromStringValue(item, is_renderer_tainted);
}
if (!url_and_title && include_files) {
diff --git a/chromium/ui/base/dragdrop/os_exchange_data_provider.h b/chromium/ui/base/dragdrop/os_exchange_data_provider.h
index b83eab4c4b9..72109093cd3 100644
--- a/chromium/ui/base/dragdrop/os_exchange_data_provider.h
+++ b/chromium/ui/base/dragdrop/os_exchange_data_provider.h
@@ -34,8 +34,12 @@ namespace ui {
class DataTransferEndpoint;
-// Controls whether or not filenames should be converted to file: URLs when
-// getting a URL.
+// Controls whether or not filenames are converted to file: URLs when getting a
+// URL. Some callers, e.g. when populating the DataTransfer object for the web
+// platform, need to suppress this conversion, as this:
+// - leaks filesystem paths to the web
+// - results in duplicate entries for the same logical entity
+// See crbug.com/40078641 for more historical context.
enum class FilenameToURLPolicy {
CONVERT_FILENAMES,
DO_NOT_CONVERT_FILENAMES,
@@ -65,6 +69,11 @@ class COMPONENT_EXPORT(UI_BASE_DATA_EXCHANGE) OSExchangeDataProvider {
const base::Pickle& data) = 0;
virtual bool GetString(std::u16string* data) const = 0;
+ // Even if there is no URL data present, many implementations will coerce text
+ // content into URLs if the text is a valid URL. This coercion should only
+ // happen for HTTP-like URLs (i.e. http or https) if the data originates from
+ // a renderer (i.e. `IsRendererTainted()` is true) to avoid bypassing the URL
+ // filtering applied when a drag is started.
virtual bool GetURLAndTitle(FilenameToURLPolicy policy,
GURL* url,
std::u16string* title) const = 0;
diff --git a/chromium/ui/base/dragdrop/os_exchange_data_provider_non_backed.cc b/chromium/ui/base/dragdrop/os_exchange_data_provider_non_backed.cc
index 8103f864a00..d92981129ae 100644
--- a/chromium/ui/base/dragdrop/os_exchange_data_provider_non_backed.cc
+++ b/chromium/ui/base/dragdrop/os_exchange_data_provider_non_backed.cc
@@ -9,6 +9,7 @@
#include "base/check.h"
#include "base/containers/contains.h"
+#include "base/feature_list.h"
#include "base/files/file_path.h"
#include "base/strings/utf_string_conversions.h"
#include "build/chromeos_buildflags.h"
@@ -17,6 +18,7 @@
#include "ui/base/clipboard/file_info.h"
#include "ui/base/data_transfer_policy/data_transfer_endpoint.h"
#include "ui/base/dragdrop/os_exchange_data.h"
+#include "ui/base/ui_base_features.h"
#include "url/gurl.h"
namespace ui {
@@ -117,8 +119,10 @@ bool OSExchangeDataProviderNonBacked::GetURLAndTitle(
std::u16string* title) const {
if ((formats_ & OSExchangeData::URL) == 0) {
title->clear();
- return GetPlainTextURL(url) ||
- (policy == FilenameToURLPolicy::CONVERT_FILENAMES &&
+ if (GetPlainTextURL(url)) {
+ return true;
+ }
+ return (policy == FilenameToURLPolicy::CONVERT_FILENAMES &&
GetFileURL(url));
}
@@ -252,6 +256,12 @@ bool OSExchangeDataProviderNonBacked::GetPlainTextURL(GURL* url) const {
if (!test_url.is_valid())
return false;
+ if (base::FeatureList::IsEnabled(
+ features::kDragDropOnlySynthesizeHttpOrHttpsUrlsFromText) &&
+ IsRendererTainted() && !test_url.SchemeIsHTTPOrHTTPS()) {
+ return false;
+ }
+
if (url)
*url = test_url;
return true;
diff --git a/chromium/ui/base/dragdrop/os_exchange_data_provider_win.cc b/chromium/ui/base/dragdrop/os_exchange_data_provider_win.cc
index a27a58d3bd4..af17b7d14f3 100644
--- a/chromium/ui/base/dragdrop/os_exchange_data_provider_win.cc
+++ b/chromium/ui/base/dragdrop/os_exchange_data_provider_win.cc
@@ -16,6 +16,7 @@
#include "base/check_op.h"
#include "base/containers/span.h"
+#include "base/feature_list.h"
#include "base/files/file_path.h"
#include "base/functional/callback.h"
#include "base/i18n/file_util_icu.h"
@@ -38,6 +39,7 @@
#include "ui/base/data_transfer_policy/data_transfer_policy_controller.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/l10n/l10n_util_win.h"
+#include "ui/base/ui_base_features.h"
#include "ui/gfx/geometry/point.h"
#include "ui/gfx/geometry/size.h"
#include "ui/gfx/image/image_skia.h"
@@ -258,6 +260,11 @@ bool OSExchangeDataProviderWin::GetPlainTextURL(IDataObject* source,
!plain_text.empty()) {
GURL gurl(plain_text);
if (gurl.is_valid()) {
+ if (base::FeatureList::IsEnabled(
+ features::kDragDropOnlySynthesizeHttpOrHttpsUrlsFromText) &&
+ IsRendererTainted() && !gurl.SchemeIsHTTPOrHTTPS()) {
+ return false;
+ }
*url = gurl;
return true;
}
diff --git a/chromium/ui/base/ui_base_features.cc b/chromium/ui/base/ui_base_features.cc
index 564a2415561..ec50c29a553 100644
--- a/chromium/ui/base/ui_base_features.cc
+++ b/chromium/ui/base/ui_base_features.cc
@@ -249,6 +249,10 @@ BASE_FEATURE(kFocusFollowsCursor,
"FocusFollowsCursor",
base::FEATURE_DISABLED_BY_DEFAULT);
+BASE_FEATURE(kDragDropOnlySynthesizeHttpOrHttpsUrlsFromText,
+ "DragDropOnlySynthesizeHttpOrHttpsUrlsFromText",
+ base::FEATURE_ENABLED_BY_DEFAULT);
+
#if BUILDFLAG(IS_WIN)
// Enables InputPane API for controlling on screen keyboard.
BASE_FEATURE(kInputPaneOnScreenKeyboard,
diff --git a/chromium/ui/base/ui_base_features.h b/chromium/ui/base/ui_base_features.h
index 67f99842c04..09d4dec9214 100644
--- a/chromium/ui/base/ui_base_features.h
+++ b/chromium/ui/base/ui_base_features.h
@@ -19,6 +19,9 @@ namespace features {
COMPONENT_EXPORT(UI_BASE_FEATURES)
BASE_DECLARE_FEATURE(kExperimentalFlingAnimation);
COMPONENT_EXPORT(UI_BASE_FEATURES) BASE_DECLARE_FEATURE(kFocusFollowsCursor);
+COMPONENT_EXPORT(UI_BASE_FEATURES)
+BASE_DECLARE_FEATURE(kDragDropOnlySynthesizeHttpOrHttpsUrlsFromText);
+
#if BUILDFLAG(IS_CHROMEOS_ASH)
COMPONENT_EXPORT(UI_BASE_FEATURES)
BASE_DECLARE_FEATURE(kSettingsShowsPerKeyboardSettings);
generated by cgit v1.2.3 (git 2.25.1) at 2025-05-10 18:09:40 +0000