Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #4173: Fixes PrivilegedRequest to work on all iOS versions. (#4174)
Browse files Browse the repository at this point in the history
* Fixed PrivilegedRequest to work on all iOS versions.

* Fix session restore due to URL not parsing in JS, but it parses correctly natively.

* Fixing session restore again.

* Perfect SessionRestoring

* Addressing Token leaking to unwanted pages.
Addressing feedback.

* Privileged requests shouldnt be added to history

* Fix Tab's title.

* Removed unused code.

Co-authored-by: Soner Yuksel <[email protected]>
Co-authored-by: Michał Buczek <[email protected]>
  • Loading branch information
3 people committed Sep 16, 2021
1 parent 98044cc commit c3163ab
Show file tree
Hide file tree
Showing 11 changed files with 177 additions and 62 deletions.
8 changes: 4 additions & 4 deletions Client.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,7 @@
CA9A233426B97B4300923D70 /* PlaylistMenuButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA9A233326B97B4300923D70 /* PlaylistMenuButton.swift */; };
CA9A234126B97B8400923D70 /* MenuButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA9A234026B97B8400923D70 /* MenuButton.swift */; };
CA550490269DED8F00C19917 /* MediaBackgrounding.js in Resources */ = {isa = PBXBuildFile; fileRef = CA55048F269DED8F00C19917 /* MediaBackgrounding.js */; };
CAA0BD9026F2562D00F14212 /* PrivilegedRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAA0BD8F26F2562D00F14212 /* PrivilegedRequest.swift */; };
CAA10597266FE94700A0372D /* RecentSearchQRCodeScannerController.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAA1058B266FE94700A0372D /* RecentSearchQRCodeScannerController.swift */; };
CAB127632639F37C00BBFC75 /* RecentSearches.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAB127622639F37C00BBFC75 /* RecentSearches.swift */; };
CAB127652639FB7000BBFC75 /* RecentSearchCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = CAB127642639FB7000BBFC75 /* RecentSearchCell.swift */; };
Expand All @@ -898,7 +899,6 @@
D308EE561CBF0BF5006843F2 /* CertError.css in Resources */ = {isa = PBXBuildFile; fileRef = D308EE551CBF0BF5006843F2 /* CertError.css */; };
D314E7F71A37B98700426A76 /* BottomToolbarView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D314E7F51A37B98700426A76 /* BottomToolbarView.swift */; };
D31A0FC71A65D6D000DC8C7E /* SearchSuggestClient.swift in Sources */ = {isa = PBXBuildFile; fileRef = D31A0FC61A65D6D000DC8C7E /* SearchSuggestClient.swift */; };
D31CF65C1CC1959A001D0BD0 /* PrivilegedRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = D31CF65B1CC1959A001D0BD0 /* PrivilegedRequest.swift */; };
D31F95E91AC226CB005C9F3B /* ScreenshotHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = D31F95E81AC226CB005C9F3B /* ScreenshotHelper.swift */; };
D34510881ACF415700EC27F0 /* SearchLoader.swift in Sources */ = {isa = PBXBuildFile; fileRef = D34510871ACF415700EC27F0 /* SearchLoader.swift */; };
D34DC8531A16C40C00D49B7B /* Profile.swift in Sources */ = {isa = PBXBuildFile; fileRef = D34DC84D1A16C40C00D49B7B /* Profile.swift */; };
Expand Down Expand Up @@ -2561,6 +2561,7 @@
CA9A233326B97B4300923D70 /* PlaylistMenuButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlaylistMenuButton.swift; sourceTree = "<group>"; };
CA9A234026B97B8400923D70 /* MenuButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MenuButton.swift; sourceTree = "<group>"; };
CA55048F269DED8F00C19917 /* MediaBackgrounding.js */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.javascript; path = MediaBackgrounding.js; sourceTree = "<group>"; };
CAA0BD8F26F2562D00F14212 /* PrivilegedRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivilegedRequest.swift; sourceTree = "<group>"; };
CAA1058B266FE94700A0372D /* RecentSearchQRCodeScannerController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = RecentSearchQRCodeScannerController.swift; sourceTree = "<group>"; };
CAB127622639F37C00BBFC75 /* RecentSearches.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RecentSearches.swift; sourceTree = "<group>"; };
CAB127642639FB7000BBFC75 /* RecentSearchCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RecentSearchCell.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2593,7 +2594,6 @@
D308EE551CBF0BF5006843F2 /* CertError.css */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.css; path = CertError.css; sourceTree = "<group>"; };
D314E7F51A37B98700426A76 /* BottomToolbarView.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = BottomToolbarView.swift; sourceTree = "<group>"; };
D31A0FC61A65D6D000DC8C7E /* SearchSuggestClient.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SearchSuggestClient.swift; sourceTree = "<group>"; };
D31CF65B1CC1959A001D0BD0 /* PrivilegedRequest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PrivilegedRequest.swift; sourceTree = "<group>"; };
D31F95E81AC226CB005C9F3B /* ScreenshotHelper.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ScreenshotHelper.swift; sourceTree = "<group>"; };
D34510871ACF415700EC27F0 /* SearchLoader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SearchLoader.swift; sourceTree = "<group>"; };
D34DC84D1A16C40C00D49B7B /* Profile.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; lineEnding = 0; path = Profile.swift; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.swift; };
Expand Down Expand Up @@ -5169,7 +5169,6 @@
7BA8D1C61BA037F500C8AE9E /* OpenInHelper.swift */,
5E5E6F9725BB658A0035B6A0 /* PlaylistHelper.swift */,
D0C95E0D200FD3B200E4E51C /* PrintHelper.swift */,
D31CF65B1CC1959A001D0BD0 /* PrivilegedRequest.swift */,
FA6B2AC11D41F02D00429414 /* Punycode.swift */,
FA9294001D6584A200AC8D33 /* QRCode.xcassets */,
FA9293D31D6580E100AC8D33 /* QRCodeViewController.swift */,
Expand Down Expand Up @@ -5250,6 +5249,7 @@
2760D2BE215ACCE20068E131 /* BundleExtensions.swift */,
0AEF99B122E22D2200294C76 /* DateGroupedTableData.swift */,
5EAB745B230456F0006055F5 /* BookmarkValidation.swift */,
CAA0BD8F26F2562D00F14212 /* PrivilegedRequest.swift */,
);
path = Shared;
sourceTree = "<group>";
Expand Down Expand Up @@ -6816,6 +6816,7 @@
files = (
5D88BB7E23ED014E001096F3 /* ImpactFeedbackGeneratorExtensions.swift in Sources */,
E65075991E37F7AB006961AC /* Cancellable.swift in Sources */,
CAA0BD9026F2562D00F14212 /* PrivilegedRequest.swift in Sources */,
E65075BE1E37F7AB006961AC /* TimeConstants.swift in Sources */,
E65075B21E37F7AB006961AC /* Functions.swift in Sources */,
E65075B41E37F7AB006961AC /* KeychainCache.swift in Sources */,
Expand Down Expand Up @@ -7237,7 +7238,6 @@
4422D43721BFD29E00BF1855 /* NSData+GZIP.m in Sources */,
0A47389E25D2DD6F0048201A /* NSPredicate+Additions.m in Sources */,
27EF6B8024BF48C7005E034F /* FeedSourceListViewController.swift in Sources */,
D31CF65C1CC1959A001D0BD0 /* PrivilegedRequest.swift in Sources */,
0A764F31230EE5CA003A1D9B /* OnboardingState.swift in Sources */,
D8D33A7D1FBD080300A20A28 /* SnapKitExtensions.swift in Sources */,
27AC7CFA24C77EBC00441317 /* FeedActionAlertView.swift in Sources */,
Expand Down
3 changes: 2 additions & 1 deletion Client/Application/WebServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class WebServer {

// For development builds, random host is used in case of working with multiple instances of the app.
// For safety we keep static port number for public builds.
let port = AppConstants.buildChannel.isPublic ? 6571 : Int.random(in: 6572..<6600)
let port = AppConstants.webServerPort

/// A random, transient token used for authenticating requests.
/// Other apps are able to make requests to our local Web server,
Expand Down Expand Up @@ -86,6 +86,7 @@ class WebServer {
if components.host == "localhost" && components.scheme == "http" {
components.port = Int(WebServer.sharedInstance.server.port)
}

return components.url
}

Expand Down
29 changes: 22 additions & 7 deletions Client/Assets/SessionRestore.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,34 @@
*/
(function () {
const SECURITY_TOKEN = "%SECURITY_TOKEN%"
const SESSION_RESTORE_KEY = "%SESSION_RESTORE_KEY%"
const SESSION_RESTORE_TOKEN = "%SESSION_RESTORE_TOKEN%"

function getRestoreURL(url) {
// If the url already points to an error page just return the url as is
if (url.indexOf(document.location.origin + '/errors/error.html') === 0) {
return url;
}
// If the url already points to an error page or session restore page,
// just return the url as is
if (url.startsWith(document.location.origin + '/errors/error.html') ||
url.startsWith(document.location.origin + '/about/sessionrestore.html')) {
const newURL = new URL(url);
newURL.searchParams.set(SESSION_RESTORE_KEY, SESSION_RESTORE_TOKEN);
return newURL.toString();
}

// Otherwise, push an error page to trigger a redirect when loaded.
return '/errors/error.html?url=' + escape(url);
//return document.location.origin + '/errors/error.html?url=' + escape(url);

const newURL = new URL(document.location.origin + '/errors/error.html');
newURL.searchParams.set('url', url);
newURL.searchParams.set(SESSION_RESTORE_KEY, SESSION_RESTORE_TOKEN);
return newURL.toString();
}
var index = document.location.href.search("history");

// Parse the query parameters
var queryParameters = new URLSearchParams(document.location.href);

// Pull the session out of the history query argument.
// The session is a JSON-stringified array of all URLs to restore for this tab, plus the last active index.
var sessionRestoreComponents = JSON.parse(unescape(document.location.href.substring(index + "history=".length)));
var sessionRestoreComponents = JSON.parse(unescape(queryParameters.get('history')));
var urlList = sessionRestoreComponents['history'];
var currentPage = sessionRestoreComponents['currentPage'];
// First, replace the session restore page (this page) with the first URL to be restored.
Expand Down
2 changes: 1 addition & 1 deletion Client/Frontend/Browser/BrowserViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1851,7 +1851,7 @@ class BrowserViewController: UIViewController {
runScriptsOnWebView(webView)

// Only add history of a url which is not a localhost url
if !tab.isPrivate {
if !tab.isPrivate, !PrivilegedRequest.isWebServerRequest(url: url) {
// The visitType is checked If it is "typed" or not to determine the History object we are adding
// should be synced or not. This limitation exists on browser side so we are aligning with this
if let visitType =
Expand Down
41 changes: 0 additions & 41 deletions Client/Frontend/Browser/PrivilegedRequest.swift

This file was deleted.

13 changes: 12 additions & 1 deletion Client/Frontend/Browser/SessionData.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class SessionData: NSObject, NSSecureCoding {
**/
init(currentPage: Int, urls: [URL], lastUsedTime: Timestamp) {
self.currentPage = currentPage
self.urls = urls
self.urls = SessionData.updateSessionURLs(urls: urls)
self.lastUsedTime = lastUsedTime

assert(!urls.isEmpty, "Session has at least one entry")
Expand Down Expand Up @@ -67,4 +67,15 @@ class SessionData: NSObject, NSSecureCoding {
static var supportsSecureCoding: Bool {
return true
}

private static func updateSessionURLs(urls: [URL]) -> [URL] {
return urls.compactMap { url in
if PrivilegedRequest.isWebServerRequest(url: url),
PrivilegedRequest.isPrivileged(url: url),
let strippedURL = PrivilegedRequest.removePrivileges(url: url) {
return strippedURL
}
return url
}
}
}
13 changes: 12 additions & 1 deletion Client/Frontend/Browser/SessionRestoreHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,16 @@ import Shared
struct SessionRestoreHandler {
static func register(_ webServer: WebServer) {
// Register the handler that accepts /about/sessionrestore?history=...&currentpage=... requests.
webServer.registerHandlerForMethod("GET", module: "about", resource: "sessionrestore") { _ in
webServer.registerHandlerForMethod("GET", module: "about", resource: "sessionrestore") { request in

// Session Restore should only ever be called from a privileged request
// This is because we need to inject the `RESTORE_TOKEN` into the page.
// Therefore, we don't want to leak it to a malicious page
guard let query = request?.query,
query[PrivilegedRequest.key] == PrivilegedRequest.token else {
return GCDWebServerResponse(statusCode: 404)
}

if let sessionRestorePath = Bundle.main.path(forResource: "SessionRestore", ofType: "html") {
do {
var sessionRestoreString = try String(contentsOfFile: sessionRestorePath)
Expand All @@ -22,6 +31,8 @@ struct SessionRestoreHandler {

let securityToken = UserScriptManager.messageHandlerToken.uuidString
sessionRestoreString = sessionRestoreString.replacingOccurrences(of: "%SECURITY_TOKEN%", with: securityToken, options: .literal)
sessionRestoreString = sessionRestoreString.replacingOccurrences(of: "%SESSION_RESTORE_KEY%", with: PrivilegedRequest.key, options: .literal)
sessionRestoreString = sessionRestoreString.replacingOccurrences(of: "%SESSION_RESTORE_TOKEN%", with: PrivilegedRequest.token, options: .literal)

return GCDWebServerDataResponse(html: sessionRestoreString)
} catch _ {}
Expand Down
25 changes: 21 additions & 4 deletions Client/Frontend/Browser/Tab.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,15 @@ class Tab: NSObject {
fileprivate var lastRequest: URLRequest?
var restoring: Bool = false
var pendingScreenshot = false
var url: URL?
var url: URL? {
didSet {
if let _url = url,
PrivilegedRequest.isWebServerRequest(url: _url),
PrivilegedRequest.isPrivileged(url: _url) {
url = PrivilegedRequest.removePrivileges(url: _url)
}
}
}
var mimeType: String?
var isEditing: Bool = false
var shouldClassifyLoadsForAds = true
Expand Down Expand Up @@ -309,8 +317,17 @@ class Tab: NSObject {
jsonDict[SessionData.Keys.history] = updatedURLs as AnyObject
jsonDict[SessionData.Keys.currentPage] = Int(currentPage) as AnyObject

guard let escapedJSON = JSON(jsonDict).rawString()?.addingPercentEncoding(withAllowedCharacters: CharacterSet.urlQueryAllowed),
let restoreURL = URL(string: "\(WebServer.sharedInstance.base)/about/sessionrestore?history=\(escapedJSON)") else {
// We escape everything, except alpha-numeric characters
// This is because when `URLSearchParams` in `SessionRestore.html`
// attempts to parse the URL query parameters, it will not properly parse
// the unescaped characters.. Therefore, we will get a blank screen,
// after restoring multiple times, because SessionRestore can be double
// percent-escaped (percent-encoded multiple times)!
// Especially if the original URL already has escaped query parameters.
let characterSet: CharacterSet = .alphanumerics

guard let escapedJSON = JSON(jsonDict).rawString()?.addingPercentEncoding(withAllowedCharacters: characterSet),
let restoreURL = URL(string: "\(WebServer.sharedInstance.base)/about/sessionrestore?timestamp=\(Int(Date().timeIntervalSince1970))&history=\(escapedJSON)") else {
return
}

Expand Down Expand Up @@ -379,7 +396,7 @@ class Tab: NSObject {
var displayTitle: String {
if let title = webView?.title, !title.isEmpty {
return title.contains("localhost") ? "" : title
} else if let url = webView?.url ?? self.url, url.isAboutHomeURL {
} else if webView?.url?.isAboutHomeURL == true || self.url?.isAboutHomeURL == true {
return Strings.newTabTitle
}

Expand Down
4 changes: 4 additions & 0 deletions Shared/AppConstants.swift
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ public struct AppConstants {
}
return scheme
}()

public static let webServerPort: Int = {
AppConstants.buildChannel.isPublic ? 6571 : Int.random(in: 6572..<6600)
}()

public static let prefSendUsageData = "settings.sendUsageData"

Expand Down
20 changes: 18 additions & 2 deletions Shared/Extensions/URLExtensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,15 @@ extension URL {

public var decodeReaderModeURL: URL? {
if self.isReaderModeURL {
if let components = URLComponents(url: self, resolvingAgainstBaseURL: false), let queryItems = components.queryItems, queryItems.count == 1 {
var url = self

// Remove Privileges from the URL
if PrivilegedRequest.isPrivileged(url: url),
let strippedURL = PrivilegedRequest.removePrivileges(url: url) {
url = strippedURL
}

if let components = URLComponents(url: url, resolvingAgainstBaseURL: false), let queryItems = components.queryItems, queryItems.count == 1 {
if let queryItem = queryItems.first, let value = queryItem.value {
return URL(string: value)
}
Expand Down Expand Up @@ -421,7 +429,15 @@ extension URL {
}

public var originalURLFromErrorURL: URL? {
let components = URLComponents(url: self, resolvingAgainstBaseURL: false)
var url = self

// Remove Privileges from the URL
if PrivilegedRequest.isPrivileged(url: url),
let strippedURL = PrivilegedRequest.removePrivileges(url: url) {
url = strippedURL
}

let components = URLComponents(url: url, resolvingAgainstBaseURL: false)
if self.isErrorPageURL, let queryURL = components?.queryItems?.find({ $0.name == "url" })?.value {
return URL(string: queryURL)
}
Expand Down
Loading

0 comments on commit c3163ab

Please sign in to comment.