Skip to content

Commit

Permalink
[ios] Suppress JS dialogs if visible URL origin differs dialog's origin
Browse files Browse the repository at this point in the history
Please see crbug.com/1029907 for details.

(cherry picked from commit a3ec688)

Bug: 1029907
TBR: gambard@chromium.org
Change-Id: I2dab884278ada0d2532bbb0adb4cf3d71fd7850e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2277069
Auto-Submit: Eugene But <eugenebut@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Commit-Queue: Eugene But <eugenebut@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#784420}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2295400
Reviewed-by: Eugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/branch-heads/4183@{#477}
Cr-Branched-From: 740e9e8-refs/heads/master@{#782793}
  • Loading branch information
Eugene But authored and Commit Bot committed Jul 13, 2020
1 parent 24a222b commit f789a96
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 1 deletion.
22 changes: 21 additions & 1 deletion ios/web/web_state/ui/crw_web_controller_unittest.mm
Expand Up @@ -378,7 +378,7 @@ void SetWebViewURL(NSString* url_string) {
}

// Test fixture to test JavaScriptDialogPresenter.
class JavaScriptDialogPresenterTest : public WebTestWithWebState {
class JavaScriptDialogPresenterTest : public WebTestWithWebController {
protected:
JavaScriptDialogPresenterTest() : page_url_("https://chromium.test/") {}
void SetUp() override {
Expand Down Expand Up @@ -468,6 +468,26 @@ void TearDown() override {
EXPECT_NSEQ(@"No", dialog->default_prompt_text);
}

// Tests that window.alert, window.confirm and window.prompt dialogs are not
// shown if URL of presenting main frame is different from visible URL.
TEST_F(JavaScriptDialogPresenterTest, DifferentVisibleUrl) {
ASSERT_TRUE(requested_dialogs().empty());

// Change visible URL.
AddPendingItem(GURL("https://pending.test/"), ui::PAGE_TRANSITION_TYPED);
web_controller().webStateImpl->SetIsLoading(true);
ASSERT_NE(page_url().GetOrigin(), web_state()->GetVisibleURL().GetOrigin());

ExecuteJavaScript(@"alert('test')");
ASSERT_TRUE(requested_dialogs().empty());

EXPECT_NSEQ(@NO, ExecuteJavaScript(@"confirm('test')"));
ASSERT_TRUE(requested_dialogs().empty());

EXPECT_NSEQ([NSNull null], ExecuteJavaScript(@"prompt('Yes?', 'No')"));
ASSERT_TRUE(requested_dialogs().empty());
}

// Test fixture for testing visible security state.
typedef WebTestWithWebState CRWWebStateSecurityStateTest;

Expand Down
12 changes: 12 additions & 0 deletions ios/web/web_state/ui/crw_wk_ui_handler.mm
Expand Up @@ -264,6 +264,18 @@ - (void)runJavaScriptDialogOfType:(web::JavaScriptDialogType)type
return;
}

if (self.webStateImpl->GetVisibleURL().GetOrigin() !=
requestURL.GetOrigin() &&
frame.mainFrame) {
// Dialog was requested by web page's main frame, but visible URL has
// different origin. This could happen if the user has started a new
// browser initiated navigation. There is no value in showing dialogs
// requested by page, which this WebState is about to leave. But presenting
// the dialog can lead to phishing and other abusive behaviors.
completionHandler(NO, nil);
return;
}

self.webStateImpl->RunJavaScriptDialog(
requestURL, type, message, defaultText,
base::BindOnce(^(bool success, NSString* input) {
Expand Down

0 comments on commit f789a96

Please sign in to comment.