Skip to content

Commit

Permalink
Fix: external application requests via redirect URLs shows wrong orig…
Browse files Browse the repository at this point in the history
…in. (#1900)

Task/Issue URL:
https://app.asana.com/0/1176047645786601/1204521390227183/f


**Description**:
If the external application URL was the result of a cross-origin
redirect, then we display the wrong origin in the external application
request popup. Instead, I propose we check if redirects occurred, and
use the origin of the most recent redirect as the first choice for the
domain displayed to the user.

This is only a proposed fix, and will likely need a thorough review to
ensure we don't cause breakage in legitimate external application
requests.

**Steps to test this PR**:
1. Visit
https://alesandroortiz.com/security/chromium/external-protocol/spoof-links.html
2. Click **Tel**
3. Check the origin in the popup is aogarantiza.com and not
alesandroortiz.com
4. Manually enter tel://155555 into address bar
5. Ensure popup appears with origin "155555"
6. Visit this test page: https://crossorigin.site/tel.html
7. Click the button
8. Ensure the origin is displayed as "crossorigin.site"

<!--
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be
marked with the `DO NOT MERGE` label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the `Pending Product
Review` label.

If at any point it isn't actively being worked on/ready for
review/otherwise moving forward (besides the above PR/PFR exception)
strongly consider closing it (or not opening it in the first place). If
you decide not to close it, make sure it's labelled to make it clear the
PRs state and comment with more information.
-->

---
###### Internal references:
[Pull Request Review
Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f)
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
[Pull Request
Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)
  • Loading branch information
not-a-rootkit authored Dec 20, 2023
1 parent d30f00b commit 19c06f2
Showing 1 changed file with 7 additions and 8 deletions.
15 changes: 7 additions & 8 deletions DuckDuckGo/Tab/Navigation/ExternalAppSchemeHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ final class ExternalAppSchemeHandler {

extension ExternalAppSchemeHandler: NavigationResponder {

@MainActor
func decidePolicy(for navigationAction: NavigationAction, preferences: inout NavigationPreferences) async -> NavigationActionPolicy? {
// swiftlint:disable:next function_body_length
@MainActor func decidePolicy(for navigationAction: NavigationAction, preferences: inout NavigationPreferences) async -> NavigationActionPolicy? {
let externalUrl = navigationAction.url
// only proceed with non-external-scheme navigations
guard externalUrl.isExternalSchemeLink, let scheme = externalUrl.scheme else {
Expand Down Expand Up @@ -114,19 +114,18 @@ extension ExternalAppSchemeHandler: NavigationResponder {
}

let permissionType = PermissionType.externalScheme(scheme: scheme)
// use domain from the url for user-entered app schemes, otherwise use current website domain
let domain = navigationAction.isUserEnteredUrl ? navigationAction.url.host ?? "" : navigationAction.sourceFrame.securityOrigin.host
permissionModel.permissions([permissionType], requestedForDomain: domain, url: externalUrl) { [workspace, weak self] isGranted in
// Check for cross-origin redirects first, then use domain from the url for user-entered app schemes, then use current website domain
let redirectDomain = navigationAction.redirectHistory?.reversed().first(where: { $0.url.host != navigationAction.url.host })?.url.host
let domain = redirectDomain ?? (navigationAction.isUserEnteredUrl ? navigationAction.url.host ?? "" : navigationAction.sourceFrame.securityOrigin.host)
permissionModel.permissions([permissionType], requestedForDomain: domain, url: externalUrl) { [workspace] isGranted in
if isGranted {
workspace.open(externalUrl)

// if "Always allow" is set and this is the only navigation in the tab: close the tab
if self?.shouldCloseTabOnExternalAppOpen == true, let webView = navigationAction.targetFrame?.webView {
if self.shouldCloseTabOnExternalAppOpen == true, let webView = navigationAction.targetFrame?.webView {
webView.close()
}
}
}

return .cancel
}

Expand Down

0 comments on commit 19c06f2

Please sign in to comment.