chore: cherry-pick 2 changes from 2-M136 (#47176)

* chore: [34-x-y] cherry-pick 2 changes from 2-M136

* 295a4a1b14b8 from chromium
* c3568ceda9d8 from chromium

* chore: update patches
This commit is contained in:
Keeley Hammond
2025-05-21 01:27:08 -07:00
committed by GitHub
parent 781a0ab416
commit a6d4fc4044
3 changed files with 369 additions and 0 deletions

View File

@@ -155,3 +155,5 @@ cherry-pick-b8f80176b163.patch
fix_false_activation_logic_for_context_menu.patch
mac_fix_check_on_ime_reconversion_due_to_invalid_replacement_range.patch
cherry-pick-0333ecde9142.patch
cherry-pick-295a4a1b14b8.patch
cherry-pick-c3568ceda9d8.patch

View File

@@ -0,0 +1,269 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Alex Gough <ajgo@chromium.org>
Date: Mon, 5 May 2025 19:09:15 -0700
Subject: Drop transitive trust from transports
Untrusted nodes could reflect a broker initiated transport back to
a broker. This ultimately allows for handle leaks if the reflected
transport was later used to deserialize another transport containing
handles in the broker.
This CL addresses this along several axes:
1. untrusted transports cannot return new links to brokers.
2. process trustiness on Windows is propagated when a transport is
deserialized from a transport.
Windows has a special additional level of trustiness associated with
mojo peers via the is_remote_process_untrusted attribute (the
MOJO_SEND_INVITATION_FLAG_UNTRUSTED_PROCESS in invitations). This
affects how handles are sent between processes. This was a bool on all
platforms which was confusing.
This CL makes this attribute clearer. On Windows it is now a bi-state
enum, while on other platforms it is simply kUntracked. This makes it
easier to use default constructed values, and the same API on all
platforms without using too many buildflag differences.
This state was not being propagated correctly during transport
deserialization, and is now set as the same trust as the process from
which a deserialized transport came. Processes currently default to
being kTrusted, which matches the current behavior of the bool flag.
Finally, this CL turns a DCHECK into a CHECK to ensure peers are only
elevated when expected.
Bug: 412578726
Change-Id: I6741a3f53b26c3df854731177cdc886e9c8f7f11
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6497400
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Alex Gough <ajgo@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1456055}
diff --git a/mojo/core/ipcz_driver/invitation.cc b/mojo/core/ipcz_driver/invitation.cc
index 4b3f717dab641bc08070dbb70aad6838fa397c63..001b146f1469579c6b6192fb16ad06e49dc045eb 100644
--- a/mojo/core/ipcz_driver/invitation.cc
+++ b/mojo/core/ipcz_driver/invitation.cc
@@ -90,7 +90,7 @@ IpczDriverHandle CreateTransportForMojoEndpoint(
base::Process remote_process = base::Process(),
MojoProcessErrorHandler error_handler = nullptr,
uintptr_t error_handler_context = 0,
- bool is_remote_process_untrusted = false) {
+ Transport::ProcessTrust remote_process_trust = Transport::ProcessTrust{}) {
CHECK_EQ(endpoint.num_platform_handles, 1u);
auto handle =
PlatformHandle::FromMojoPlatformHandle(&endpoint.platform_handles[0]);
@@ -100,7 +100,7 @@ IpczDriverHandle CreateTransportForMojoEndpoint(
auto transport = base::MakeRefCounted<Transport>(
endpoint_types, PlatformChannelEndpoint(std::move(handle)),
- std::move(remote_process), is_remote_process_untrusted);
+ std::move(remote_process), remote_process_trust);
transport->SetErrorHandler(error_handler, error_handler_context);
transport->set_leak_channel_on_shutdown(options.leak_channel_on_shutdown);
transport->set_is_peer_trusted(options.is_peer_trusted);
@@ -268,15 +268,21 @@ MojoResult Invitation::Send(
// bit essentially means that the remote process is especially untrustworthy
// (e.g. a Chrome renderer) and should be subject to additional constraints
// regarding what types of objects can be transferred to it.
- const bool is_remote_process_untrusted =
- options &&
- (options->flags & MOJO_SEND_INVITATION_FLAG_UNTRUSTED_PROCESS) != 0;
+ Transport::ProcessTrust remote_process_trust{};
+#if BUILDFLAG(IS_WIN)
+ if (options &&
+ (options->flags & MOJO_SEND_INVITATION_FLAG_UNTRUSTED_PROCESS) != 0) {
+ remote_process_trust = Transport::ProcessTrust::kUntrusted;
+ } else {
+ remote_process_trust = Transport::ProcessTrust::kTrusted;
+ }
+#endif
const bool is_peer_elevated =
options && (options->flags & MOJO_SEND_INVITATION_FLAG_ELEVATED);
#if !BUILDFLAG(IS_WIN)
// For now, the concept of an elevated process is only meaningful on Windows.
- DCHECK(!is_peer_elevated);
+ CHECK(!is_peer_elevated);
#endif
#if BUILDFLAG(IS_WIN)
@@ -296,7 +302,7 @@ MojoResult Invitation::Send(
*transport_endpoint,
{.is_peer_trusted = is_peer_elevated, .is_trusted_by_peer = true},
std::move(remote_process), error_handler, error_handler_context,
- is_remote_process_untrusted);
+ remote_process_trust);
if (transport == IPCZ_INVALID_DRIVER_HANDLE) {
return MOJO_RESULT_INVALID_ARGUMENT;
}
diff --git a/mojo/core/ipcz_driver/transport.cc b/mojo/core/ipcz_driver/transport.cc
index d13778473bb88d71f1d9452f61de0aee81f53d41..94d616a9cf2bad4662dded1332ef3f632a692d4b 100644
--- a/mojo/core/ipcz_driver/transport.cc
+++ b/mojo/core/ipcz_driver/transport.cc
@@ -135,7 +135,7 @@ bool EncodeHandle(PlatformHandle& handle,
const base::Process& remote_process,
HandleOwner handle_owner,
HandleData& out_handle_data,
- bool is_remote_process_untrusted) {
+ Transport::ProcessTrust remote_process_trust) {
CHECK(handle.is_valid());
// Duplicating INVALID_HANDLE_VALUE passes a process handle. If you intend to
// do this, you must open a valid process handle, not pass the result of
@@ -157,7 +157,7 @@ bool EncodeHandle(PlatformHandle& handle,
DCHECK_EQ(handle_owner, HandleOwner::kRecipient);
DCHECK(remote_process.IsValid());
#if BUILDFLAG(IS_WIN)
- if (is_remote_process_untrusted) {
+ if (remote_process_trust == Transport::ProcessTrust::kUntrusted) {
DcheckIfFileHandleIsUnsafe(handle.GetHandle().get());
}
#endif
@@ -218,23 +218,20 @@ scoped_refptr<base::SingleThreadTaskRunner>& GetIOTaskRunnerStorage() {
Transport::Transport(EndpointTypes endpoint_types,
PlatformChannelEndpoint endpoint,
base::Process remote_process,
- bool is_remote_process_untrusted)
+ ProcessTrust remote_process_trust)
: endpoint_types_(endpoint_types),
remote_process_(std::move(remote_process)),
-#if BUILDFLAG(IS_WIN)
- is_remote_process_untrusted_(is_remote_process_untrusted),
-#endif
- inactive_endpoint_(std::move(endpoint)) {
-}
+ remote_process_trust_(remote_process_trust),
+ inactive_endpoint_(std::move(endpoint)) {}
// static
scoped_refptr<Transport> Transport::Create(EndpointTypes endpoint_types,
PlatformChannelEndpoint endpoint,
base::Process remote_process,
- bool is_remote_process_untrusted) {
+ ProcessTrust remote_process_trust) {
return base::MakeRefCounted<Transport>(endpoint_types, std::move(endpoint),
std::move(remote_process),
- is_remote_process_untrusted);
+ remote_process_trust);
}
// static
@@ -473,7 +470,7 @@ IpczResult Transport::SerializeObject(ObjectBase& object,
for (size_t i = 0; i < object_num_handles; ++i) {
#if BUILDFLAG(IS_WIN)
ok &= EncodeHandle(platform_handles[i], remote_process_, handle_owner,
- handle_data[i], is_remote_process_untrusted_);
+ handle_data[i], remote_process_trust());
#else
handles[i] = TransmissiblePlatformHandle::ReleaseAsHandle(
base::MakeRefCounted<TransmissiblePlatformHandle>(
@@ -635,23 +632,39 @@ scoped_refptr<Transport> Transport::Deserialize(
process = base::Process(handles[1].ReleaseHandle());
}
#endif
+ // Reject transports with out of range enum value in destination_type.
+ if (!(header.destination_type == kBroker ||
+ header.destination_type == kNonBroker)) {
+ return nullptr;
+ }
+
const bool is_source_trusted = from_transport.is_peer_trusted() ||
from_transport.destination_type() == kBroker;
+
const bool is_new_peer_trusted = header.is_peer_trusted;
+ const bool is_trusted_by_peer = header.is_trusted_by_peer;
+
if (is_new_peer_trusted && !is_source_trusted) {
// Untrusted transports cannot send us trusted transports.
return nullptr;
}
+
+ if (header.destination_type == kBroker && !is_source_trusted) {
+ // Do not accept broker connections from untrusted transports.
+ return nullptr;
+ }
+
if (header.is_same_remote_process &&
from_transport.remote_process().IsValid()) {
process = from_transport.remote_process().Duplicate();
}
- auto transport = Create({.source = from_transport.source_type(),
- .destination = header.destination_type},
- PlatformChannelEndpoint(std::move(handles[0])),
- std::move(process));
+ auto transport =
+ Create({.source = from_transport.source_type(),
+ .destination = header.destination_type},
+ PlatformChannelEndpoint(std::move(handles[0])), std::move(process),
+ from_transport.remote_process_trust());
transport->set_is_peer_trusted(is_new_peer_trusted);
- transport->set_is_trusted_by_peer(header.is_trusted_by_peer);
+ transport->set_is_trusted_by_peer(is_trusted_by_peer);
// Inherit the IO task used by the receiving Transport. Deserialized
// transports are always adopted by the receiving node, and we want any given
diff --git a/mojo/core/ipcz_driver/transport.h b/mojo/core/ipcz_driver/transport.h
index ae400f392ec2ff3f83c2a037d53d541d7b223b4b..564604750b5b73d3ec199c92e66ef5e55aa64b46 100644
--- a/mojo/core/ipcz_driver/transport.h
+++ b/mojo/core/ipcz_driver/transport.h
@@ -36,6 +36,18 @@ class MOJO_SYSTEM_IMPL_EXPORT Transport : public Object<Transport>,
kNonBroker,
};
+ // Is the remote process trusted, only tracked on Windows. Not directly
+ // sent over the wire.
+ enum class ProcessTrust : uint32_t {
+#if BUILDFLAG(IS_WIN)
+ // Default to kTrusted. TODO(crbug.com/414392683) - invert this.
+ kTrusted,
+ kUntrusted,
+#else
+ kUntracked,
+#endif
+ };
+
struct EndpointTypes {
EndpointType source;
EndpointType destination;
@@ -43,7 +55,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Transport : public Object<Transport>,
Transport(EndpointTypes endpoint_types,
PlatformChannelEndpoint endpoint,
base::Process remote_process,
- bool is_remote_process_untrusted = false);
+ ProcessTrust remote_process_trust);
// Static helper that is slightly more readable due to better type deduction
// than MakeRefCounted<T>.
@@ -51,7 +63,7 @@ class MOJO_SYSTEM_IMPL_EXPORT Transport : public Object<Transport>,
EndpointTypes endpoint_types,
PlatformChannelEndpoint endpoint,
base::Process remote_process = base::Process(),
- bool is_remote_process_untrusted = false);
+ ProcessTrust remote_process_trust = ProcessTrust{});
static std::pair<scoped_refptr<Transport>, scoped_refptr<Transport>>
CreatePair(EndpointType first_type, EndpointType second_type);
@@ -84,6 +96,8 @@ class MOJO_SYSTEM_IMPL_EXPORT Transport : public Object<Transport>,
void set_is_trusted_by_peer(bool trusted) { is_trusted_by_peer_ = trusted; }
bool is_trusted_by_peer() const { return is_trusted_by_peer_; }
+ ProcessTrust remote_process_trust() const { return remote_process_trust_; }
+
void SetErrorHandler(MojoProcessErrorHandler handler, uintptr_t context) {
error_handler_ = handler;
error_handler_context_ = context;
@@ -203,12 +217,10 @@ class MOJO_SYSTEM_IMPL_EXPORT Transport : public Object<Transport>,
// meaningless on platforms other than Windows.
bool is_trusted_by_peer_ = false;
-#if BUILDFLAG(IS_WIN)
// Indicates whether the remote process is "untrusted" in Mojo parlance,
// meaning this Transport restricts what kinds of objects can be transferred
- // from this end (Windows only.)
- bool is_remote_process_untrusted_;
-#endif
+ // from this end (kTrusted or kUntrusted on Windows, kUntracked elsewhere.)
+ const ProcessTrust remote_process_trust_;
// The channel endpoint which will be used by this Transport to construct and
// start its underlying Channel instance once activated. Not guarded by a lock

View File

@@ -0,0 +1,98 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Takashi Nakayama <tnak@chromium.org>
Date: Thu, 8 May 2025 04:57:11 -0700
Subject: Set `referrerpolicy: "no-referrer"` in link loads from subresources
This CL overwrites the referrerpolicy attribute on Link header in sub-
resource loads.
See the bug for details.
Bug: 415810136
Change-Id: I750e1043ecbd2ce63e827cdbdd2a2a22661ffea7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6522070
Commit-Queue: Takashi Nakayama <tnak@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1457510}
diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc
index 1b2b6128e69953be7f9f96853d76c5c0e16552fe..0796e273ecf510292f808fb146658d1e96c6aeb1 100644
--- a/third_party/blink/common/features.cc
+++ b/third_party/blink/common/features.cc
@@ -2864,7 +2864,10 @@ BASE_FEATURE(kReleaseResourceDecodedDataOnMemoryPressure,
"ReleaseResourceDecodedDataOnMemoryPressure",
base::FEATURE_ENABLED_BY_DEFAULT);
-// When adding new features or constants for features, please keep the features
+BASE_FEATURE(kNoReferrerForPreloadFromSubresource,
+ "NoReferrerForPreloadFromSubresource",
+ base::FEATURE_ENABLED_BY_DEFAULT);
+
// sorted by identifier name (e.g. `kAwesomeFeature`), and the constants for
// that feature grouped with the associated feature.
//
diff --git a/third_party/blink/public/common/features.h b/third_party/blink/public/common/features.h
index 0bc9302cd8f3f291949be328aba5df88e2ab0d03..4d8855be76776c390a65c40094c51a13940ff792 100644
--- a/third_party/blink/public/common/features.h
+++ b/third_party/blink/public/common/features.h
@@ -1832,6 +1832,9 @@ BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(
BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(
kReleaseResourceDecodedDataOnMemoryPressure);
+// Kill switch for https://crbug.com/415810136.
+BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kNoReferrerForPreloadFromSubresource);
+
// When adding new features or constants for features, please keep the features
// sorted by identifier name (e.g. `kAwesomeFeature`), and the constants for
// that feature grouped with the associated feature.
diff --git a/third_party/blink/renderer/core/loader/preload_helper.cc b/third_party/blink/renderer/core/loader/preload_helper.cc
index 3ffce30776c56991b41103b098ec981db8143a5d..630147649907abdd279e06d283b6d282908c8dc0 100644
--- a/third_party/blink/renderer/core/loader/preload_helper.cc
+++ b/third_party/blink/renderer/core/loader/preload_helper.cc
@@ -4,6 +4,7 @@
#include "third_party/blink/renderer/core/loader/preload_helper.h"
+#include "base/feature_list.h"
#include "base/metrics/histogram_functions.h"
#include "base/timer/elapsed_timer.h"
#include "third_party/blink/public/common/features.h"
@@ -246,6 +247,23 @@ bool IsCompressionDictionaryLoadAllowed(
}
}
+bool IsSubresourceLoad(PreloadHelper::LoadLinksFromHeaderMode mode) {
+ switch (mode) {
+ case PreloadHelper::LoadLinksFromHeaderMode::kDocumentBeforeCommit:
+ case PreloadHelper::LoadLinksFromHeaderMode::
+ kDocumentAfterCommitWithoutViewport:
+ case PreloadHelper::LoadLinksFromHeaderMode::
+ kDocumentAfterCommitWithViewport:
+ case PreloadHelper::LoadLinksFromHeaderMode::kDocumentAfterLoadCompleted:
+ return false;
+ case PreloadHelper::LoadLinksFromHeaderMode::kSubresourceFromMemoryCache:
+ case PreloadHelper::LoadLinksFromHeaderMode::kSubresourceNotFromMemoryCache:
+ return true;
+ default:
+ NOTREACHED();
+ }
+}
+
} // namespace
void PreloadHelper::DnsPrefetchIfNeeded(
@@ -795,6 +813,15 @@ void PreloadHelper::LoadLinksFromHeader(
LinkLoadParameters params(header, base_url);
bool change_rel_to_prefetch = false;
+ // For security purposes, set `referrerpolicy: "no-referrer"` in link loads
+ // from subresources. See https://crbug.com/415810136 for details.
+ if (base::FeatureList::IsEnabled(
+ blink::features::kNoReferrerForPreloadFromSubresource)) {
+ if (IsSubresourceLoad(mode)) {
+ params.referrer_policy = network::mojom::ReferrerPolicy::kNever;
+ }
+ }
+
if (params.rel.IsLinkPreload() && recursive_prefetch_token) {
// Only preload headers are expected to have a recursive prefetch token
// In response to that token's existence, we treat the request as a