Skip to content

Commit 35caa7c

Browse files
antosartmibrunin
authored andcommitted
[Backport] CVE-2021-21175: Inappropriate implementation in Site isolation
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/2731577: Strip url to origin in X-Frame-Options violation messages X-Frame-Options violations are logged via a console message in the parent frame. To avoid leaking sensitive data to the parent frame, let's report as "blocked url" just the origin of the blocked frame's url, as we are already doing for the frame-ancestors CSP directive. [M86 Merge]: ancestor_throttle.cc was moved. (cherry picked from commit 93ce5606cd9a9597993ba70670b4092ab6722281) Bug: 1146651 Change-Id: If5e5ac62f7e44e714b109e6adc389f11999e0f8b Commit-Queue: Antonio Sartori <[email protected]> Reviewed-by: Charlie Reis <[email protected]> Reviewed-by: Arthur Sonzogni <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#828651} Reviewed-by: Achuith Bhandarkar <[email protected]> Commit-Queue: Victor-Gabriel Savu <[email protected]> Cr-Commit-Position: refs/branch-heads/4240@{#1563} Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218} Reviewed-by: Allan Sandfeld Jensen <[email protected]> Reviewed-by: Jüri Valdmann <[email protected]>
1 parent 5466811 commit 35caa7c

File tree

1 file changed

+19
-3
lines changed

1 file changed

+19
-3
lines changed

chromium/content/browser/frame_host/ancestor_throttle.cc

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,12 +227,20 @@ void AncestorThrottle::ParseError(const std::string& value,
227227
"Refused to display '%s' in a frame because it set multiple "
228228
"'X-Frame-Options' headers with conflicting values "
229229
"('%s'). Falling back to 'deny'.",
230-
navigation_handle()->GetURL().spec().c_str(), value.c_str());
230+
url::Origin::Create(navigation_handle()->GetURL())
231+
.GetURL()
232+
.spec()
233+
.c_str(),
234+
value.c_str());
231235
} else {
232236
message = base::StringPrintf(
233237
"Invalid 'X-Frame-Options' header encountered when loading '%s': "
234238
"'%s' is not a recognized directive. The header will be ignored.",
235-
navigation_handle()->GetURL().spec().c_str(), value.c_str());
239+
url::Origin::Create(navigation_handle()->GetURL())
240+
.GetURL()
241+
.spec()
242+
.c_str(),
243+
value.c_str());
236244
}
237245

238246
// Log a console error in the parent of the current RenderFrameHost (as
@@ -250,11 +258,19 @@ void AncestorThrottle::ConsoleError(HeaderDisposition disposition) {
250258
std::string message = base::StringPrintf(
251259
"Refused to display '%s' in a frame because it set 'X-Frame-Options' "
252260
"to '%s'.",
253-
navigation_handle()->GetURL().spec().c_str(),
261+
url::Origin::Create(navigation_handle()->GetURL())
262+
.GetURL()
263+
.spec()
264+
.c_str(),
254265
disposition == HeaderDisposition::DENY ? "deny" : "sameorigin");
255266

256267
// Log a console error in the parent of the current RenderFrameHost (as
257268
// the current RenderFrameHost itself doesn't yet have a document).
269+
//
270+
// TODO(https://crbug.com/1146651): We should not leak any information at all
271+
// to the parent frame. Send a message directly to Devtools instead (without
272+
// passing through a renderer): that can also contain more information (like
273+
// the full blocked url).
258274
navigation_handle()->GetRenderFrameHost()->GetParent()->AddMessageToConsole(
259275
CONSOLE_MESSAGE_LEVEL_ERROR, message);
260276
}

0 commit comments

Comments
 (0)