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

Fix #4208 - Reader-Mode Flaw #4209

Merged
merged 3 commits into from
Sep 24, 2021
Merged

Fix #4208 - Reader-Mode Flaw #4209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bbb72b6
Readermode `Hook` fix.
Brandon-T Sep 22, 2021
77d59ee
Use `textContent` for escaping and preventing XSS.
Brandon-T Sep 22, 2021
6f66750
Addressed feedback
Brandon-T Sep 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions Client/Frontend/Reader/Reader.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,18 @@
<meta name="viewport" content="width=device-width, user-scalable=no, minimum-scale=.25, maximum-scale=1.6, initial-scale=1.0">
<meta name="referrer" content="never">
<link rel="stylesheet" type="text/css" href="/reader-mode/styles/Reader.css">
<title>%READER-TITLE%</title>
<title id="reader-page-title"></title>
</head>

<body data-readerStyle='%READER-STYLE%'>
<div id="reader-header" class="header">
<h1 id="reader-title">%READER-TITLE%</h1>
<h1 id="reader-title"></h1>
<div id="reader-credits" class="credits">%READER-CREDITS%</div>
<script type="text/javascript" src="/reader-mode/javascript/Reader.js"></script>
<script nonce="%READER-TITLE-NONCE%">
document.getElementById("reader-page-title").textContent = "%READER-TITLE%";
document.getElementById("reader-title").textContent = "%READER-TITLE%";
</script>
</div>

<div id="reader-content" class="content">
Expand Down
4 changes: 2 additions & 2 deletions Client/Frontend/Reader/ReaderMode.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ struct ReadabilityResult {
self.content = content
}
if let title = dict["title"] as? String {
self.title = title
self.title = title.htmlEntityEncodedString
}
if let credits = dict["byline"] as? String {
self.credits = credits
Expand All @@ -182,7 +182,7 @@ struct ReadabilityResult {
let domain = object["domain"].string
let url = object["url"].string
let content = object["content"].string
let title = object["title"].string
let title = object["title"].string?.htmlEntityEncodedString
let credits = object["credits"].string

if domain == nil || url == nil || content == nil || title == nil || credits == nil {
Expand Down
9 changes: 7 additions & 2 deletions Client/Frontend/Reader/ReaderModeHandlers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,15 @@ struct ReaderModeHandlers {
readerModeStyle = style
}
}
if let html = ReaderModeUtils.generateReaderContent(readabilityResult, initialStyle: readerModeStyle),

// Must generate a unique nonce, every single time as per Content-Policy spec.
let setTitleNonce = UUID().uuidString.replacingOccurrences(of: "-", with: "")

if let html = ReaderModeUtils.generateReaderContent(readabilityResult, initialStyle: readerModeStyle,
titleNonce: setTitleNonce),
let response = GCDWebServerDataResponse(html: html) {
// Apply a Content Security Policy that disallows everything except images from anywhere and fonts and css from our internal server
response.setValue("default-src 'none'; img-src *; style-src http://localhost:* '\(readerModeStyleHash)'; font-src http://localhost:*", forAdditionalHeader: "Content-Security-Policy")
response.setValue("default-src 'none'; img-src *; style-src http://localhost:* '\(readerModeStyleHash)'; font-src http://localhost:*; script-src 'nonce-\(setTitleNonce)'", forAdditionalHeader: "Content-Security-Policy")
return response
}
} catch _ {
Expand Down
3 changes: 2 additions & 1 deletion Client/Frontend/Reader/ReaderModeUtils.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ struct ReaderModeUtils {
} ?? domain
}

static func generateReaderContent(_ readabilityResult: ReadabilityResult, initialStyle: ReaderModeStyle) -> String? {
static func generateReaderContent(_ readabilityResult: ReadabilityResult, initialStyle: ReaderModeStyle, titleNonce: String) -> String? {
guard let stylePath = Bundle.main.path(forResource: "Reader", ofType: "css"),
let css = try? String(contentsOfFile: stylePath, encoding: .utf8),
let tmplPath = Bundle.main.path(forResource: "Reader", ofType: "html"),
Expand All @@ -27,6 +27,7 @@ struct ReaderModeUtils {
.replacingOccurrences(of: "%READER-TITLE%", with: readabilityResult.title)
.replacingOccurrences(of: "%READER-CREDITS%", with: readabilityResult.credits)
.replacingOccurrences(of: "%READER-CONTENT%", with: readabilityResult.content)
.replacingOccurrences(of: "%READER-TITLE-NONCE%", with: titleNonce)

}
}
14 changes: 0 additions & 14 deletions Client/Frontend/UserContent/UserScripts/MainFrame/AtDocumentStart/ReaderMode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,6 @@ function checkReadability() {
var readability = new Readability(uri, doc, { debug: DEBUG });
readabilityResult = readability.parse();

// Sanitize the title to prevent a malicious page from inserting HTML in the `<title>`.
if (readabilityResult && readabilityResult.title !== undefined) {
readabilityResult.title = escapeHTML(readabilityResult.title);
}

debug({Type: "ReaderModeStateChange", Value: readabilityResult !== null ? "Available" : "Unavailable"});
webkit.messageHandlers.readerModeMessageHandler.postMessage({"securitytoken": SECURITY_TOKEN, "data": {Type: "ReaderModeStateChange", Value: readabilityResult !== null ? "Available" : "Unavailable"}});
webkit.messageHandlers.readerModeMessageHandler.postMessage({"securitytoken": SECURITY_TOKEN, "data": {Type: "ReaderContentParsed", Value: readabilityResult}});
Expand All @@ -76,15 +71,6 @@ function checkReadability() {
}, 100);
}

function escapeHTML(string) {
return string
.replace(/\&/g, "&amp;")
.replace(/\</g, "&lt;")
.replace(/\>/g, "&gt;")
.replace(/\"/g, "&quot;")
.replace(/\'/g, "&#039;");
}

// Readerize the document. Since we did the actual readerization already in checkReadability, we
// can simply return the results we already have.
function readerize() {
Expand Down