diff --git a/Package.swift b/Package.swift index 0e8e48d..6c506f5 100644 --- a/Package.swift +++ b/Package.swift @@ -5,7 +5,9 @@ let package = Package( name: "Reader", platforms: [.macOS(.v13)], dependencies: [ - .package(url: "https://github.com/sparkle-project/Sparkle", from: "2.6.4"), + // Pinned to an exact version to prevent a future compromised Sparkle + // release from being silently consumed on the next `swift package update`. + .package(url: "https://github.com/sparkle-project/Sparkle", exact: "2.6.4"), ], targets: [ .executableTarget( diff --git a/Resources/Info.plist b/Resources/Info.plist index 0daba23..6f4851d 100644 --- a/Resources/Info.plist +++ b/Resources/Info.plist @@ -64,10 +64,19 @@ SUEnableInstallerLauncherService + SUFeedURL - https://raw.githubusercontent.com/thejefflarson/reader/main/appcast.xml + https://thejefflarson.github.io/reader/appcast.xml SUPublicEDKey zBk/+O7F6ZvuCdEM7p7FQ3VHdkOFkRbJhMbZGRjqP3U= + + NSAppTransportSecurity + + NSAllowsArbitraryLoads + + UTImportedTypeDeclarations diff --git a/Sources/Reader/AppDelegate.swift b/Sources/Reader/AppDelegate.swift index fe4dfbd..3cf1181 100644 --- a/Sources/Reader/AppDelegate.swift +++ b/Sources/Reader/AppDelegate.swift @@ -29,7 +29,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate { // MARK: - File handling func application(_ sender: NSApplication, openFile filename: String) -> Bool { - openURL(URL(fileURLWithPath: filename)) + // Validate the extension before accepting the path. CLI callers bypass + // NSOpenPanel's content-type filter, so we must enforce it here to + // prevent arbitrary files from being fed to the editor / Recent Documents. + let url = URL(fileURLWithPath: filename) + let allowedExtensions: Set = ["md", "markdown", "mdown", "mkd", "mkdn"] + guard allowedExtensions.contains(url.pathExtension.lowercased()) else { return false } + openURL(url) return true } diff --git a/Sources/Reader/EditorTextView.swift b/Sources/Reader/EditorTextView.swift index 39ad481..6bb4549 100644 --- a/Sources/Reader/EditorTextView.swift +++ b/Sources/Reader/EditorTextView.swift @@ -283,11 +283,17 @@ final class EditorTextView: NSTextView, NSTextStorageDelegate { // For HTML payloads the pasteboard almost always also contains a // plain-text representation; we take that path and refuse to // parse HTML. - if let plain = pboard.string(forType: .string) { + // Cap plain-text paste to 1 MB to prevent main-thread stall on restyle(). + let maxPasteChars = 1 * 1024 * 1024 + if let plain = pboard.string(forType: .string), plain.count <= maxPasteChars { insertText(plain, replacementRange: selectedRange()) return true } + // Cap RTF data to 4 MB before handing to NSAttributedString's parser, + // which allocates memory proportional to the input inside AppKit. + let maxRTFBytes = 4 * 1024 * 1024 if let data = pboard.data(forType: .rtf), + data.count <= maxRTFBytes, let attr = NSAttributedString(rtf: data, documentAttributes: nil) { insertText(MarkdownSerializer.markdown(from: attr), replacementRange: selectedRange()) return true diff --git a/Sources/Reader/MainWindowController.swift b/Sources/Reader/MainWindowController.swift index c3f3252..d7ea93a 100644 --- a/Sources/Reader/MainWindowController.swift +++ b/Sources/Reader/MainWindowController.swift @@ -190,10 +190,23 @@ final class MainWindowController: NSWindowController, NSWindowDelegate { func open(url: URL) { do { + // Reject files larger than 10 MB to prevent an unbounded main-thread + // allocation. This also guards against symlinks to /dev/zero or huge + // sparse files, which String(contentsOf:) would read indefinitely. + let maxFileBytes = 10 * 1024 * 1024 + let attrs = try FileManager.default.attributesOfItem(atPath: url.path) + if let size = attrs[.size] as? Int, size > maxFileBytes { + throw CocoaError(.fileReadTooLarge) + } let content = try String(contentsOf: url, encoding: .utf8) currentFileURL = url markdown = content - NSDocumentController.shared.noteNewRecentDocumentURL(url) + // Only record markdown files in Recent Documents; arbitrary paths + // (e.g. CLI-supplied /etc/passwd) must not appear in the system list. + let allowedExtensions: Set = ["md", "markdown", "mdown", "mkd", "mkdn"] + if allowedExtensions.contains(url.pathExtension.lowercased()) { + NSDocumentController.shared.noteNewRecentDocumentURL(url) + } // Opened documents land in preview mode — a file you received // is something you read first, not something you're drafting. // New blank windows (⌘N) stay in edit mode. diff --git a/Sources/Reader/MarkdownSerializer.swift b/Sources/Reader/MarkdownSerializer.swift index 635ff10..d60d757 100644 --- a/Sources/Reader/MarkdownSerializer.swift +++ b/Sources/Reader/MarkdownSerializer.swift @@ -22,10 +22,20 @@ enum MarkdownSerializer { var content = text + // Only embed URLs whose schemes are safe for markdown links written to + // disk. RTF clipboard content can carry file://, app://, x-apple-*, + // and other schemes that should never be serialised into a .md file. + let allowedSchemes: Set = ["http", "https", "mailto"] if let url = attributes[.link] as? URL { + guard let scheme = url.scheme?.lowercased(), allowedSchemes.contains(scheme) else { + return content + } return "[\(content)](\(url.absoluteString))" } if let urlString = attributes[.link] as? String, let url = URL(string: urlString) { + guard let scheme = url.scheme?.lowercased(), allowedSchemes.contains(scheme) else { + return content + } return "[\(content)](\(url.absoluteString))" } diff --git a/Sources/Reader/MarkdownStyler.swift b/Sources/Reader/MarkdownStyler.swift index acc03db..95c6f29 100644 --- a/Sources/Reader/MarkdownStyler.swift +++ b/Sources/Reader/MarkdownStyler.swift @@ -23,6 +23,11 @@ final class MarkdownStyler { func restyle(_ storage: NSTextStorage) { let full = NSRange(location: 0, length: storage.length) guard full.length > 0 else { return } + // Refuse to run all regex rules synchronously on very large documents — + // that can produce a multi-second main-thread stall (and is the first + // link in the ReDoS chain). 1 MB is well above any realistic note size. + let maxStyleBytes = 1 * 1024 * 1024 + guard storage.length <= maxStyleBytes else { return } storage.setAttributes(baseAttributes(), range: full) let occupied = NSMutableIndexSet() @@ -253,7 +258,9 @@ final class MarkdownStyler { private static func setextHeading2() -> Rule { // `Title\n---` — but only if the title line isn't itself a list // bullet or blockquote (which would fight this interpretation). - let pattern = regex("(?m)^([^\\n]+)\\n(-{2,})\\s*$") + // Use [ \t]* instead of \s* to avoid O(n²) backtracking on lines + // ending with many hyphens that are not followed by a newline. + let pattern = regex("(?m)^([^\\n]+)\\n(-{2,})[ \\t]*$") return Rule(pattern: pattern, occupies: false) { match, ctx in let titleLine = match.range(at: 1) let titleText = (ctx.storage.string as NSString).substring(with: titleLine) @@ -348,7 +355,11 @@ final class MarkdownStyler { // Match any line that looks like a table row: starts and ends with // `|`, has at least one interior `|`. Cells are monospace so columns // align visually in the source-is-rendered model. - let pattern = regex("(?m)^(\\s*\\|.+\\|)\\s*$") + // + // Pattern uses (?:\|[^|\n]+)+ instead of .+ to eliminate O(n²) + // backtracking: each cell is unambiguously "|", + // so the engine can never re-split a match when the trailing | is absent. + let pattern = regex("(?m)^([ \\t]*(?:\\|[^|\\n]+)+\\|)[ \\t]*$") return Rule(pattern: pattern, occupies: false) { match, ctx in let whole = match.range ctx.storage.addAttribute(.font, value: Theme.codeFont, range: whole) @@ -376,9 +387,13 @@ final class MarkdownStyler { // MARK: - Inline emphasis private static func bold() -> Rule { + // Do NOT use .dotMatchesLineSeparators here: with that flag and an + // unclosed ** marker, the lazy .+? expands to span the entire document, + // causing catastrophic backtracking on every keystroke. Constraining + // to [^\n] limits each match to a single line, making the rule O(n). let pattern = try! NSRegularExpression( - pattern: "(\\*\\*|__)(?=\\S)(.+?)(?<=\\S)\\1", - options: [.dotMatchesLineSeparators] + pattern: "(\\*\\*|__)(?=\\S)([^\\n]+?)(?<=\\S)\\1", + options: [] ) return Rule(pattern: pattern, occupies: false) { match, ctx in let whole = match.range diff --git a/appcast.xml b/appcast.xml index 9feb346..a294d7c 100644 --- a/appcast.xml +++ b/appcast.xml @@ -4,7 +4,11 @@ xmlns:dc="http://purl.org/dc/elements/1.1/"> Reader - https://raw.githubusercontent.com/thejefflarson/reader/main/appcast.xml + + https://thejefflarson.github.io/reader/appcast.xml Reader updates. en diff --git a/scripts/reader b/scripts/reader index 0dae740..1793d21 100755 --- a/scripts/reader +++ b/scripts/reader @@ -35,6 +35,17 @@ for arg in "$@"; do echo "reader: $arg: no such file" >&2 exit 1 fi + # Validate the file extension before forwarding to Launch Services. + # Any existing file would otherwise be opened by Reader, bypassing the + # content-type filter that NSOpenPanel enforces for interactive opens. + ext="${arg##*.}" + case "$ext" in + md|markdown|mdown|mkd|mkdn) ;; + *) + echo "reader: $arg: not a supported markdown file (.$ext)" >&2 + exit 1 + ;; + esac abs="$(/usr/bin/dirname "$arg")" abs="$(cd "$abs" && pwd)/$(/usr/bin/basename "$arg")" absolute_paths="$absolute_paths diff --git a/scripts/release.sh b/scripts/release.sh index 9022131..f510c8a 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -19,6 +19,12 @@ if [ -z "$VERSION" ]; then echo "usage: $0 [\"release notes\"]" exit 1 fi +# Validate version format to prevent shell metacharacters and XML special +# characters from being injected into Info.plist or appcast.xml. +if ! printf '%s' "$VERSION" | grep -qE '^[0-9]+\.[0-9]+(\.[0-9]+)?(-[A-Za-z0-9]+)?$'; then + echo "error: VERSION must match X.Y[.Z][-qualifier] (e.g. 1.2.0), got: $VERSION" + exit 1 +fi : "${NOTES:=Reader $VERSION}" REPO="thejefflarson/reader" @@ -43,14 +49,19 @@ cp Resources/AppIcon.icns "$APP_DIR/Contents/Resources/AppIcon.icns" # Patch Info.plist with the version at release time, then copy in. /usr/libexec/PlistBuddy \ - -c "Set :CFBundleShortVersionString $VERSION" \ - -c "Set :CFBundleVersion $VERSION" \ - Resources/Info.plist >/dev/null 2>&1 || true + -c "Set :CFBundleShortVersionString ${VERSION}" \ + -c "Set :CFBundleVersion ${VERSION}" \ + Resources/Info.plist cp Resources/Info.plist "$APP_DIR/Contents/Info.plist" printf 'APPL????' > "$APP_DIR/Contents/PkgInfo" -# Ad-hoc sign so Gatekeeper will at least see a signature. -codesign --force --deep --sign - "$APP_DIR" 2>/dev/null || true +# Sign with a real Developer ID so Gatekeeper accepts the app without +# quarantine prompts. Ad-hoc signing (--sign -) produces a binary that +# is indistinguishable from a tampered copy and is rejected by Gatekeeper +# on first launch on any machine other than the build machine. +# Set SIGN_IDENTITY in the environment to override (e.g. for a specific team). +SIGN_IDENTITY="${SIGN_IDENTITY:-Developer ID Application}" +codesign --force --deep --options runtime --sign "$SIGN_IDENTITY" "$APP_DIR" echo "==> zipping $APP_DIR" /usr/bin/ditto -c -k --sequesterRsrc --keepParent "$APP_DIR" "$ZIP_PATH" @@ -67,13 +78,17 @@ fi DATE="$(/bin/date -u +"%a, %d %b %Y %H:%M:%S +0000")" DOWNLOAD_URL="https://github.com/$REPO/releases/download/v$VERSION/$ZIP_NAME" +# Escape any ]]> sequences in NOTES so they cannot close the CDATA section +# early and inject arbitrary XML into the appcast served to all users. +NOTES_SAFE="${NOTES//]]>/]]]]>}" + ITEM=" Version $VERSION $DATE $VERSION $VERSION 13.0 - + Reader - https://raw.githubusercontent.com/$REPO/main/appcast.xml + https://thejefflarson.github.io/reader/appcast.xml Reader updates. en $ITEM @@ -101,14 +116,18 @@ else import sys, re path, item = sys.argv[1], sys.argv[2] text = open(path).read() -text = re.sub( +# Use re.subn so a structural mismatch (no insertion point found) is a hard +# error rather than a silent no-op that leaves the appcast unmodified. +new_text, n = re.subn( r"(.*?\s*[^<]*\s*)", lambda m: m.group(1) + item + "\n", text, count=1, flags=re.DOTALL, ) -open(path, "w").write(text) +if n == 0: + sys.exit("error: could not locate insertion point in " + path) +open(path, "w").write(new_text) PY fi