Commit f53af59e authored by Ali Juma's avatar Ali Juma Committed by Chromium LUCI CQ

[iOS] Disallow tel: navigations for cross-origin target frames

This CL disallows navigations to tel: URLs when the target frame
is cross-origin with respect to the frame that is initiating the
navigation.

Change-Id: If2ce2d464065414513ed646e8611a919e5339f45
Bug: 1103119
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2627751Reviewed-by: default avatarMohammad Refaat <mrefaat@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Commit-Queue: Ali Juma <ajuma@chromium.org>
Cr-Commit-Position: refs/heads/master@{#844251}
parent 4eab15e1
...@@ -184,6 +184,12 @@ AppLauncherTabHelper::ShouldAllowRequest( ...@@ -184,6 +184,12 @@ AppLauncherTabHelper::ShouldAllowRequest(
} }
} }
// Disallow navigations to tel: URLs from cross-origin frames.
if (request_url.SchemeIs(url::kTelScheme) &&
request_info.target_frame_is_cross_origin) {
return web::WebStatePolicyDecider::PolicyDecision::Cancel();
}
ExternalURLRequestStatus request_status = ExternalURLRequestStatus request_status =
ExternalURLRequestStatus::kMainFrameRequestAllowed; ExternalURLRequestStatus::kMainFrameRequestAllowed;
// TODO(crbug.com/852489): Check if the source frame should also be // TODO(crbug.com/852489): Check if the source frame should also be
......
...@@ -44,6 +44,7 @@ class ITunesUrlsHandlerTabHelperTest : public PlatformTest { ...@@ -44,6 +44,7 @@ class ITunesUrlsHandlerTabHelperTest : public PlatformTest {
fake_launcher_.launchedProductParams = nil; fake_launcher_.launchedProductParams = nil;
web::WebStatePolicyDecider::RequestInfo request_info( web::WebStatePolicyDecider::RequestInfo request_info(
ui::PageTransition::PAGE_TRANSITION_LINK, main_frame, ui::PageTransition::PAGE_TRANSITION_LINK, main_frame,
/*target_frame_is_cross_origin=*/false,
/*has_user_gesture=*/false); /*has_user_gesture=*/false);
web::WebStatePolicyDecider::PolicyDecision request_policy = web::WebStatePolicyDecider::PolicyDecision request_policy =
web_state_.ShouldAllowRequest( web_state_.ShouldAllowRequest(
......
...@@ -69,7 +69,8 @@ class SafeBrowsingTabHelperTest ...@@ -69,7 +69,8 @@ class SafeBrowsingTabHelperTest
ui::PageTransition transition = ui::PageTransition transition =
ui::PageTransition::PAGE_TRANSITION_FIRST) { ui::PageTransition::PAGE_TRANSITION_FIRST) {
web::WebStatePolicyDecider::RequestInfo request_info( web::WebStatePolicyDecider::RequestInfo request_info(
transition, for_main_frame, /*has_user_gesture=*/false); transition, for_main_frame, /*target_frame_is_cross_origin=*/false,
/*has_user_gesture=*/false);
return web_state_.ShouldAllowRequest( return web_state_.ShouldAllowRequest(
[NSURLRequest requestWithURL:net::NSURLWithGURL(url)], request_info); [NSURLRequest requestWithURL:net::NSURLWithGURL(url)], request_info);
} }
......
...@@ -30,9 +30,11 @@ class InvalidUrlTabHelperTest : public PlatformTest { ...@@ -30,9 +30,11 @@ class InvalidUrlTabHelperTest : public PlatformTest {
ui::PageTransition transition) { ui::PageTransition transition) {
NSURL* url = [NSURL URLWithString:spec]; NSURL* url = [NSURL URLWithString:spec];
NSURLRequest* request = [[NSURLRequest alloc] initWithURL:url]; NSURLRequest* request = [[NSURLRequest alloc] initWithURL:url];
web::WebStatePolicyDecider::RequestInfo info(transition, web::WebStatePolicyDecider::RequestInfo info(
/*target_frame_is_main=*/true, transition,
/*has_user_gesture=*/false); /*target_frame_is_main=*/true,
/*target_frame_is_cross_origin=*/false,
/*has_user_gesture=*/false);
return web_state_.ShouldAllowRequest(request, info); return web_state_.ShouldAllowRequest(request, info);
} }
......
...@@ -40,6 +40,7 @@ ...@@ -40,6 +40,7 @@
#import "ios/web/web_state/web_state_impl.h" #import "ios/web/web_state/web_state_impl.h"
#include "ios/web/web_view/content_type_util.h" #include "ios/web/web_view/content_type_util.h"
#import "ios/web/web_view/error_translation_util.h" #import "ios/web/web_view/error_translation_util.h"
#import "ios/web/web_view/wk_security_origin_util.h"
#import "ios/web/web_view/wk_web_view_util.h" #import "ios/web/web_view/wk_web_view_util.h"
#import "net/base/mac/url_conversions.h" #import "net/base/mac/url_conversions.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
...@@ -391,8 +392,19 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation( ...@@ -391,8 +392,19 @@ void ReportOutOfSyncURLInDidStartProvisionalNavigation(
self.userInteractionState->HasUserTappedRecently(webView) && self.userInteractionState->HasUserTappedRecently(webView) &&
net::GURLWithNSURL(action.request.mainDocumentURL) == net::GURLWithNSURL(action.request.mainDocumentURL) ==
self.userInteractionState->LastUserInteraction()->main_document_url; self.userInteractionState->LastUserInteraction()->main_document_url;
BOOL isCrossOriginTargetFrame = NO;
if (action.sourceFrame && action.targetFrame &&
action.sourceFrame != action.targetFrame) {
url::Origin sourceOrigin =
url::Origin::Create(web::GURLOriginWithWKSecurityOrigin(
action.sourceFrame.securityOrigin));
url::Origin targetOrigin =
url::Origin::Create(web::GURLOriginWithWKSecurityOrigin(
action.targetFrame.securityOrigin));
isCrossOriginTargetFrame = !sourceOrigin.IsSameOriginWith(targetOrigin);
}
web::WebStatePolicyDecider::RequestInfo requestInfo( web::WebStatePolicyDecider::RequestInfo requestInfo(
transition, isMainFrameNavigationAction, transition, isMainFrameNavigationAction, isCrossOriginTargetFrame,
userInteractedWithRequestMainFrame); userInteractedWithRequestMainFrame);
policyDecision = policyDecision =
......
...@@ -84,15 +84,20 @@ class WebStatePolicyDecider { ...@@ -84,15 +84,20 @@ class WebStatePolicyDecider {
struct RequestInfo { struct RequestInfo {
RequestInfo(ui::PageTransition transition_type, RequestInfo(ui::PageTransition transition_type,
bool target_frame_is_main, bool target_frame_is_main,
bool target_frame_is_cross_origin,
bool has_user_gesture) bool has_user_gesture)
: transition_type(transition_type), : transition_type(transition_type),
target_frame_is_main(target_frame_is_main), target_frame_is_main(target_frame_is_main),
target_frame_is_cross_origin(target_frame_is_cross_origin),
has_user_gesture(has_user_gesture) {} has_user_gesture(has_user_gesture) {}
// The navigation page transition type. // The navigation page transition type.
ui::PageTransition transition_type = ui::PageTransition transition_type =
ui::PageTransition::PAGE_TRANSITION_FIRST; ui::PageTransition::PAGE_TRANSITION_FIRST;
// Indicates whether the navigation target frame is the main frame. // Indicates whether the navigation target frame is the main frame.
bool target_frame_is_main = false; bool target_frame_is_main = false;
// Indicates whether the navigation target frame is cross-origin with
// respect to the the navigation source frame.
bool target_frame_is_cross_origin = false;
// Indicates if there was a recent user interaction with the request frame. // Indicates if there was a recent user interaction with the request frame.
bool has_user_gesture = false; bool has_user_gesture = false;
}; };
......
...@@ -15,6 +15,7 @@ namespace web { ...@@ -15,6 +15,7 @@ namespace web {
FakeShouldAllowRequestInfo::FakeShouldAllowRequestInfo() FakeShouldAllowRequestInfo::FakeShouldAllowRequestInfo()
: request_info(ui::PageTransition::PAGE_TRANSITION_FIRST, : request_info(ui::PageTransition::PAGE_TRANSITION_FIRST,
/*target_frame_is_main=*/false, /*target_frame_is_main=*/false,
/*target_frame_is_cross_origin=*/false,
/*has_user_gesture=*/false) {} /*has_user_gesture=*/false) {}
FakeShouldAllowRequestInfo::~FakeShouldAllowRequestInfo() = default; FakeShouldAllowRequestInfo::~FakeShouldAllowRequestInfo() = default;
......
...@@ -46,6 +46,14 @@ bool WaitForWebViewNotContainingText( ...@@ -46,6 +46,14 @@ bool WaitForWebViewNotContainingText(
NSTimeInterval timeout = base::test::ios::kWaitForPageLoadTimeout) NSTimeInterval timeout = base::test::ios::kWaitForPageLoadTimeout)
WARN_UNUSED_RESULT; WARN_UNUSED_RESULT;
// Waits for the given web state to have a frame that contains |text|. If the
// condition is not met within |timeout| false is returned.
bool WaitForWebViewContainingTextInFrame(
web::WebState* web_state,
std::string text,
NSTimeInterval timeout = base::test::ios::kWaitForPageLoadTimeout)
WARN_UNUSED_RESULT;
// Waits for a web view with the corresponding |image_id| and |image_state|, in // Waits for a web view with the corresponding |image_id| and |image_state|, in
// the given |web_state|. // the given |web_state|.
bool WaitForWebViewContainingImage(std::string image_id, bool WaitForWebViewContainingImage(std::string image_id,
......
...@@ -148,6 +148,15 @@ bool WaitForWebViewNotContainingText(web::WebState* web_state, ...@@ -148,6 +148,15 @@ bool WaitForWebViewNotContainingText(web::WebState* web_state,
}); });
} }
bool WaitForWebViewContainingTextInFrame(web::WebState* web_state,
std::string text,
NSTimeInterval timeout) {
return WaitUntilConditionOrTimeout(timeout, ^{
base::RunLoop().RunUntilIdle();
return IsWebViewContainingTextInFrame(web_state, text);
});
}
bool WaitForWebViewContainingImage(std::string image_id, bool WaitForWebViewContainingImage(std::string image_id,
web::WebState* web_state, web::WebState* web_state,
ImageStateElement image_state) { ImageStateElement image_state) {
......
...@@ -622,6 +622,7 @@ TEST_F(WebStateImplTest, PolicyDeciderTest) { ...@@ -622,6 +622,7 @@ TEST_F(WebStateImplTest, PolicyDeciderTest) {
WebStatePolicyDecider::RequestInfo request_info_main_frame( WebStatePolicyDecider::RequestInfo request_info_main_frame(
ui::PageTransition::PAGE_TRANSITION_LINK, ui::PageTransition::PAGE_TRANSITION_LINK,
/*target_main_frame=*/true, /*target_main_frame=*/true,
/*target_frame_is_cross_origin=*/false,
/*has_user_gesture=*/false); /*has_user_gesture=*/false);
EXPECT_CALL(decider, ShouldAllowRequest( EXPECT_CALL(decider, ShouldAllowRequest(
request, RequestInfoMatch(request_info_main_frame))) request, RequestInfoMatch(request_info_main_frame)))
...@@ -640,6 +641,7 @@ TEST_F(WebStateImplTest, PolicyDeciderTest) { ...@@ -640,6 +641,7 @@ TEST_F(WebStateImplTest, PolicyDeciderTest) {
WebStatePolicyDecider::RequestInfo request_info_iframe( WebStatePolicyDecider::RequestInfo request_info_iframe(
ui::PageTransition::PAGE_TRANSITION_LINK, ui::PageTransition::PAGE_TRANSITION_LINK,
/*target_main_frame=*/false, /*target_main_frame=*/false,
/*target_frame_is_cross_origin=*/false,
/*has_user_gesture=*/false); /*has_user_gesture=*/false);
EXPECT_CALL(decider, ShouldAllowRequest( EXPECT_CALL(decider, ShouldAllowRequest(
request, RequestInfoMatch(request_info_iframe))) request, RequestInfoMatch(request_info_iframe)))
......
...@@ -35,9 +35,11 @@ TEST_F(WebStatePolicyDeciderBridgeTest, ShouldAllowRequest) { ...@@ -35,9 +35,11 @@ TEST_F(WebStatePolicyDeciderBridgeTest, ShouldAllowRequest) {
NSURLRequest* request = [NSURLRequest requestWithURL:url]; NSURLRequest* request = [NSURLRequest requestWithURL:url];
ui::PageTransition transition_type = ui::PageTransition::PAGE_TRANSITION_LINK; ui::PageTransition transition_type = ui::PageTransition::PAGE_TRANSITION_LINK;
bool target_frame_is_main = true; bool target_frame_is_main = true;
bool target_frame_is_cross_origin = false;
bool has_user_gesture = false; bool has_user_gesture = false;
WebStatePolicyDecider::RequestInfo request_info( WebStatePolicyDecider::RequestInfo request_info(
transition_type, target_frame_is_main, has_user_gesture); transition_type, target_frame_is_main, target_frame_is_cross_origin,
has_user_gesture);
decider_bridge_.ShouldAllowRequest(request, request_info); decider_bridge_.ShouldAllowRequest(request, request_info);
FakeShouldAllowRequestInfo* should_allow_request_info = FakeShouldAllowRequestInfo* should_allow_request_info =
[decider_ shouldAllowRequestInfo]; [decider_ shouldAllowRequestInfo];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment