[Extensions] Display window only when its bounds intersect the displays
Windows should only be displayed when they intersect the displays in a
meaningful manner (in this case, window should be visible at least 50%).
Otherwise, the window will be displayed in the current default.
This prevents for windows to be created or moved outside the displays,
and potentially been exploited.
Screencast: https://drive.google.com/file/d/1dTJdC1OtQek4n6ok0sOfccbEr8H0Ah3U/view?usp=sharing
Bug: 1301203
Change-Id: I306d837d12a79a99039e5336792a72cbe20ea57d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3536833
Reviewed-by: Mike Wasserman <msw@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Commit-Queue: Emilia Paz <emiliapaz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986515}
diff --git a/chrome/browser/extensions/api/tabs/tabs_api.cc b/chrome/browser/extensions/api/tabs/tabs_api.cc
index 0521e28..c09878e 100644
--- a/chrome/browser/extensions/api/tabs/tabs_api.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_api.cc
@@ -108,6 +108,8 @@
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/base/models/list_selection_model.h"
#include "ui/base/ui_base_types.h"
+#include "ui/display/screen.h"
+#include "ui/gfx/geometry/rect.h"
#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "chrome/browser/ui/ash/window_pin_util.h"
@@ -425,6 +427,18 @@
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
}
+// Returns whether the given `bounds` intersect with at least 50% of all the
+// displays.
+bool WindowBoundsIntersectDisplays(const gfx::Rect& bounds) {
+ int intersect_area = 0;
+ for (const auto& display : display::Screen::GetScreen()->GetAllDisplays()) {
+ gfx::Rect display_bounds = display.bounds();
+ display_bounds.Intersect(bounds);
+ intersect_area += display_bounds.size().GetArea();
+ }
+ return intersect_area >= (bounds.size().GetArea() / 2);
+}
+
} // namespace
void ZoomModeToZoomSettings(ZoomController::ZoomMode zoom_mode,
@@ -711,18 +725,28 @@
WindowSizer::GetBrowserWindowBoundsAndShowState(
gfx::Rect(), nullptr, &window_bounds, &ignored_show_state);
- // Any part of the bounds can optionally be set by the caller.
- if (create_data->left)
+ // Update the window bounds if the bounds from the create parameters
+ // intersect the displays.
+ bool set_window_bounds = false;
+ if (create_data->left) {
window_bounds.set_x(*create_data->left);
-
- if (create_data->top)
+ set_window_bounds = true;
+ }
+ if (create_data->top) {
window_bounds.set_y(*create_data->top);
-
- if (create_data->width)
+ set_window_bounds = true;
+ }
+ if (create_data->width) {
window_bounds.set_width(*create_data->width);
-
- if (create_data->height)
+ set_window_bounds = true;
+ }
+ if (create_data->height) {
window_bounds.set_height(*create_data->height);
+ set_window_bounds = true;
+ }
+
+ if (set_window_bounds && !WindowBoundsIntersectDisplays(window_bounds))
+ return RespondNow(Error(tabs_constants::kInvalidWindowBoundsError));
if (create_data->focused)
focused = *create_data->focused;
@@ -868,37 +892,38 @@
// Before changing any of a window's state, validate the update parameters.
// This prevents Chrome from performing "half" an update.
+
+ // Update the window bounds if the bounds from the update parameters intersect
+ // the displays.
+ gfx::Rect window_bounds = browser->window()->IsMinimized()
+ ? browser->window()->GetRestoredBounds()
+ : browser->window()->GetBounds();
+ bool set_window_bounds = false;
+ if (params->update_info.left) {
+ window_bounds.set_x(*params->update_info.left);
+ set_window_bounds = true;
+ }
+ if (params->update_info.top) {
+ window_bounds.set_y(*params->update_info.top);
+ set_window_bounds = true;
+ }
+ if (params->update_info.width) {
+ window_bounds.set_width(*params->update_info.width);
+ set_window_bounds = true;
+ }
+ if (params->update_info.height) {
+ window_bounds.set_height(*params->update_info.height);
+ set_window_bounds = true;
+ }
+
+ if (set_window_bounds && !WindowBoundsIntersectDisplays(window_bounds))
+ return RespondNow(Error(tabs_constants::kInvalidWindowBoundsError));
+
ui::WindowShowState show_state =
ConvertToWindowShowState(params->update_info.state);
- gfx::Rect bounds = browser->window()->IsMinimized()
- ? browser->window()->GetRestoredBounds()
- : browser->window()->GetBounds();
- bool set_bounds = false;
-
- // Any part of the bounds can optionally be set by the caller.
- if (params->update_info.left) {
- bounds.set_x(*params->update_info.left);
- set_bounds = true;
- }
-
- if (params->update_info.top) {
- bounds.set_y(*params->update_info.top);
- set_bounds = true;
- }
-
- if (params->update_info.width) {
- bounds.set_width(*params->update_info.width);
- set_bounds = true;
- }
-
- if (params->update_info.height) {
- bounds.set_height(*params->update_info.height);
- set_bounds = true;
- }
-
- if (set_bounds && (show_state == ui::SHOW_STATE_MINIMIZED ||
- show_state == ui::SHOW_STATE_MAXIMIZED ||
- show_state == ui::SHOW_STATE_FULLSCREEN)) {
+ if (set_window_bounds && (show_state == ui::SHOW_STATE_MINIMIZED ||
+ show_state == ui::SHOW_STATE_MAXIMIZED ||
+ show_state == ui::SHOW_STATE_FULLSCREEN)) {
return RespondNow(Error(tabs_constants::kInvalidWindowStateError));
}
@@ -956,10 +981,10 @@
break;
}
- if (set_bounds) {
+ if (set_window_bounds) {
// TODO(varkha): Updating bounds during a drag can cause problems and a more
// general solution is needed. See http://crbug.com/251813 .
- browser->window()->SetBounds(bounds);
+ browser->window()->SetBounds(window_bounds);
}
if (params->update_info.focused) {
diff --git a/chrome/browser/extensions/api/tabs/tabs_constants.cc b/chrome/browser/extensions/api/tabs/tabs_constants.cc
index 3f068eb5..d76698f 100644
--- a/chrome/browser/extensions/api/tabs/tabs_constants.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_constants.cc
@@ -108,6 +108,9 @@
const char kSupportedInWindowsOnlyError[] = "Supported in Windows only";
const char kInvalidWindowTypeError[] = "Invalid value for type";
const char kInvalidWindowStateError[] = "Invalid value for state";
+const char kInvalidWindowBoundsError[] =
+ "Invalid value for bounds. Bounds must be at least 50% within visible "
+ "screen space.";
const char kScreenshotsDisabled[] = "Taking screenshots has been disabled";
const char kScreenshotsDisabledByDlp[] =
"Administrator policy disables screen capture when confidential content is "
diff --git a/chrome/browser/extensions/api/tabs/tabs_constants.h b/chrome/browser/extensions/api/tabs/tabs_constants.h
index 49c6dbd5..9d580b8 100644
--- a/chrome/browser/extensions/api/tabs/tabs_constants.h
+++ b/chrome/browser/extensions/api/tabs/tabs_constants.h
@@ -103,6 +103,7 @@
extern const char kSupportedInWindowsOnlyError[];
extern const char kInvalidWindowTypeError[];
extern const char kInvalidWindowStateError[];
+extern const char kInvalidWindowBoundsError[];
extern const char kScreenshotsDisabled[];
extern const char kScreenshotsDisabledByDlp[];
extern const char kCannotUpdateMuteCaptured[];
diff --git a/chrome/browser/extensions/api/tabs/tabs_test.cc b/chrome/browser/extensions/api/tabs/tabs_test.cc
index f7917c18..50ef223 100644
--- a/chrome/browser/extensions/api/tabs/tabs_test.cc
+++ b/chrome/browser/extensions/api/tabs/tabs_test.cc
@@ -65,6 +65,8 @@
#include "pdf/buildflags.h"
#include "third_party/blink/public/common/page/page_zoom.h"
#include "ui/base/window_open_disposition.h"
+#include "ui/display/display.h"
+#include "ui/display/screen.h"
#include "ui/gfx/geometry/rect.h"
#if BUILDFLAG(IS_MAC)
@@ -754,6 +756,66 @@
keys::kInvalidWindowStateError));
}
+IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, InvalidUpdateWindowBounds) {
+ scoped_refptr<const Extension> extension(ExtensionBuilder("Test").Build());
+
+ // Get the display bounds so we can test whether the window intersects.
+ gfx::Rect displays;
+ for (const auto& display : display::Screen::GetScreen()->GetAllDisplays())
+ displays.Union(display.bounds());
+
+ int window_id = ExtensionTabUtil::GetWindowId(browser());
+ gfx::Rect window_bounds = browser()->window()->GetBounds();
+
+ static const char kArgsUpdateFunction[] = "[%u, {\"left\": %d, \"top\": %d}]";
+ // We use a small value to move the window outside or inside the bounds.
+ int window_offset = window_bounds.size().width() * 0.1;
+
+ {
+ // Window bounds that do not intersect with the display are not valid.
+ int window_left = displays.right() + window_offset;
+ int window_top = displays.bottom() + window_offset;
+ auto function = base::MakeRefCounted<WindowsUpdateFunction>();
+ function->set_extension(extension.get());
+ EXPECT_TRUE(base::MatchPattern(
+ utils::RunFunctionAndReturnError(
+ function.get(),
+ base::StringPrintf(kArgsUpdateFunction, window_id, window_left,
+ window_top),
+ browser()),
+ keys::kInvalidWindowBoundsError));
+ }
+
+ {
+ // Window bounds that intersect less than 50% with the display are not
+ // valid.
+ int window_left = displays.right() - window_offset;
+ int window_top = displays.bottom() - window_offset;
+ auto function = base::MakeRefCounted<WindowsUpdateFunction>();
+ function->set_extension(extension.get());
+ EXPECT_TRUE(base::MatchPattern(
+ utils::RunFunctionAndReturnError(
+ function.get(),
+ base::StringPrintf(kArgsUpdateFunction, window_id, window_left,
+ window_top),
+ browser()),
+ keys::kInvalidWindowBoundsError));
+ }
+
+ {
+ // Window bounds that intersect 50% or more with the display are valid.
+ int window_left = displays.right() - window_bounds.width() + window_offset;
+ int window_top = displays.bottom() - window_bounds.height() + window_offset;
+ auto function = base::MakeRefCounted<WindowsUpdateFunction>();
+ function->set_extension(extension.get());
+ EXPECT_TRUE(
+ utils::RunFunction(function.get(),
+ base::StringPrintf(kArgsUpdateFunction, window_id,
+ window_left, window_top),
+ browser(), api_test_utils::NONE));
+ }
+}
+
IN_PROC_BROWSER_TEST_F(ExtensionTabsTest, UpdateDevToolsWindow) {
DevToolsWindow* devtools = DevToolsWindowTesting::OpenDevToolsWindowSync(
browser()->tab_strip_model()->GetWebContentsAt(0), false /* is_docked */);
@@ -890,6 +952,56 @@
keys::kInvalidWindowStateError));
}
+IN_PROC_BROWSER_TEST_F(ExtensionWindowCreateTest, ValidateCreateWindowBounds) {
+ // Get the display bounds so we can test whether the window intersects.
+ gfx::Rect displays;
+ for (const auto& display : display::Screen::GetScreen()->GetAllDisplays())
+ displays.Union(display.bounds());
+
+ static const char kArgsCreateFunction[] =
+ "[{\"left\": %d, \"top\": %d, \"width\": %d, \"height\": %d }]";
+ int window_width = 100;
+ int window_height = 100;
+ // We use a small value to move the window outside or inside the bounds.
+ int window_offset = 10;
+
+ {
+ // Window bounds that do not intersect with the display are not valid.
+ int window_left = displays.right() + window_offset;
+ int window_top = displays.bottom() + window_offset;
+ EXPECT_TRUE(
+ base::MatchPattern(RunCreateWindowExpectError(base::StringPrintf(
+ kArgsCreateFunction, window_left, window_top,
+ window_width, window_height)),
+ keys::kInvalidWindowBoundsError));
+ }
+
+ {
+ // Window bounds that intersect less than 50% with the display are not
+ // valid.
+ int window_left = displays.right() - window_offset;
+ int window_top = displays.bottom() - window_offset;
+ EXPECT_TRUE(
+ base::MatchPattern(RunCreateWindowExpectError(base::StringPrintf(
+ kArgsCreateFunction, window_left, window_top,
+ window_width, window_height)),
+ keys::kInvalidWindowBoundsError));
+ }
+
+ {
+ // Window bounds that intersect 50% or more with the display are valid.
+ int window_left = displays.right() - window_width + window_offset;
+ int window_top = displays.bottom() - window_height + window_offset;
+ auto function = base::MakeRefCounted<WindowsCreateFunction>();
+ function->set_extension(ExtensionBuilder("Test").Build().get());
+ EXPECT_TRUE(utils::RunFunction(
+ function.get(),
+ base::StringPrintf(kArgsCreateFunction, window_left, window_top,
+ window_width, window_height),
+ browser(), api_test_utils::NONE));
+ }
+}
+
IN_PROC_BROWSER_TEST_F(ExtensionWindowCreateTest, CreatePopupWindowFromWebUI) {
scoped_refptr<WindowsCreateFunction> function(new WindowsCreateFunction());
function->SetBrowserContextForTesting(browser()->profile());
