Skip to content

Harden RichContentFormatter sanitization + add a comprehensive test suite #25985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jkmassel merged 5 commits into from
Sep 3, 2026
Merged

Harden RichContentFormatter sanitization + add a comprehensive test suite #25985

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
84224c0
Fix grapheme-vs-UTF-16 range bug in RichContentFormatter
jkmassel Jul 23, 2026
db04c6a
Migrate RichContentFormatter tests to Swift Testing
jkmassel Sep 2, 2026
1235488
Harden parseValueForAttribute against a missing closing quote
jkmassel Sep 2, 2026
541611e
Harden RichContentFormatter sanitization beyond the UTF-16 range fix
jkmassel Sep 3, 2026
7553149
Add a comprehensive RichContentFormatter sanitization test suite
jkmassel Sep 3, 2026
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
70 changes: 35 additions & 35 deletions Modules/Sources/WordPressShared/Utility/RichContentFormatter.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import Foundation
///
public struct RegEx {
// Forbidden tags
static let styleTags = try! NSRegularExpression(pattern: "<style[^>]*?>[\\s\\S]*?</style>", options: .caseInsensitive)
static let scriptTags = try! NSRegularExpression(pattern: "<script[^>]*?>[\\s\\S]*?</script>", options: .caseInsensitive)
static let styleTags = try! NSRegularExpression(pattern: "<style[^>]*?>[\\s\\S]*?(?:</style>|$)", options: .caseInsensitive)
static let scriptTags = try! NSRegularExpression(pattern: "<script[^>]*?>[\\s\\S]*?(?:</script>|$)", options: .caseInsensitive)
static let gutenbergComments = try! NSRegularExpression(pattern: "<p><!-- /?wp:.+? /?--></p>[\\n]?", options: .caseInsensitive)

// Normalizaing Paragraphs
Expand All @@ -19,10 +19,10 @@ import Foundation
static let pTagsEnd = try! NSRegularExpression(pattern: "</p>\\s*</p>", options: .caseInsensitive)
static let newLines = try! NSRegularExpression(pattern: "\\n", options: .caseInsensitive)
static let preTags = try! NSRegularExpression(pattern: "<pre[^>]*>[\\s\\S]*?</pre>", options: .caseInsensitive)
static let videoTags = try! NSRegularExpression(pattern: "<video[^>]*>", options: .caseInsensitive)
static let videoTags = try! NSRegularExpression(pattern: "<video(\\s[^>]*)?>", options: .caseInsensitive)

// Inline Styles
static let styleAttr = try! NSRegularExpression(pattern: "\\s*style=\"[^\"]*\"", options: .caseInsensitive)
static let styleAttr = try! NSRegularExpression(pattern: "\\s+style=(?:\"[^\"]*\"|'[^']*')", options: .caseInsensitive)

// Gallery Images
public static let galleryImgTags = try! NSRegularExpression(pattern: "<img[^>]*data-orig-file[^>]*/>", options: .caseInsensitive)
Expand Down Expand Up @@ -50,17 +50,17 @@ import Foundation

content = RegEx.styleTags.stringByReplacingMatches(in: content,
options: .reportCompletion,
range: NSRange(location: 0, length: content.count),
range: NSRange(location: 0, length: content.utf16.count),
withTemplate: "")

content = RegEx.scriptTags.stringByReplacingMatches(in: content,
options: .reportCompletion,
range: NSRange(location: 0, length: content.count),
range: NSRange(location: 0, length: content.utf16.count),
withTemplate: "")

content = RegEx.gutenbergComments.stringByReplacingMatches(in: content,
options: .reportCompletion,
range: NSRange(location: 0, length: content.count),
range: NSRange(location: 0, length: content.utf16.count),
withTemplate: "")

return content
Expand All @@ -84,23 +84,23 @@ import Foundation
// Convert div tags to p tags
content = RegEx.divTagsStart.stringByReplacingMatches(in: content,
options: .reportCompletion,
range: NSRange(location: 0, length: content.count),
range: NSRange(location: 0, length: content.utf16.count),
withTemplate: openPTag)

content = RegEx.divTagsEnd.stringByReplacingMatches(in: content,
options: .reportCompletion,
range: NSRange(location: 0, length: content.count),
range: NSRange(location: 0, length: content.utf16.count),
withTemplate: closePTag)

// Remove duplicate/redundant p tags.
content = RegEx.pTagsStart.stringByReplacingMatches(in: content,
options: .reportCompletion,
range: NSRange(location: 0, length: content.count),
range: NSRange(location: 0, length: content.utf16.count),
withTemplate: openPTag)

content = RegEx.pTagsEnd.stringByReplacingMatches(in: content,
options: .reportCompletion,
range: NSRange(location: 0, length: content.count),
range: NSRange(location: 0, length: content.utf16.count),
withTemplate: closePTag)

content = filterNewLines(content)
Expand All @@ -114,11 +114,11 @@ import Foundation
var ranges = [NSRange]()
// We don't want to remove new lines from preformatted tag blocks,
// so get the ranges of such blocks.
let matches = RegEx.preTags.matches(in: content, options: .reportCompletion, range: NSRange(location: 0, length: content.count))
let matches = RegEx.preTags.matches(in: content, options: .reportCompletion, range: NSRange(location: 0, length: content.utf16.count))
if matches.isEmpty {

// No blocks found, so we'll parse the whole string.
ranges.append(NSRange(location: 0, length: content.count))
ranges.append(NSRange(location: 0, length: content.utf16.count))
} else {

// One or more preformatted blocks found, we don't want to remove new lines
Expand All @@ -133,7 +133,7 @@ import Foundation
location = match.range.location + match.range.length
}

length = content.count - location
length = content.utf16.count - location
ranges.append(NSRange(location: location, length: length))
}

Expand Down Expand Up @@ -163,7 +163,7 @@ import Foundation

content = RegEx.styleAttr.stringByReplacingMatches(in: content,
options: .reportCompletion,
range: NSRange(location: 0, length: content.count),
range: NSRange(location: 0, length: content.utf16.count),
withTemplate: "")

return content
Expand All @@ -178,19 +178,20 @@ import Foundation
/// - Returns: The value for the attribute or an empty string..
///
@objc public class func parseValueForAttribute(_ attribute: String, inElement element: String) -> String {
let elementStr = element as NSString
var value = ""
let attrStr = "\(attribute)=\""
let attrRange = elementStr.range(of: attrStr)

if attrRange.location != NSNotFound {
let location = attrRange.location + attrRange.length
let length = elementStr.length - location
let ending = elementStr.range(of: "\"", options: .caseInsensitive, range: NSRange(location: location, length: length))
value = elementStr.substring(with: NSRange(location: location, length: ending.location - location))
// Match the attribute name on a word boundary (so "rc" does not match inside "src")
// and capture its double-quoted value. A missing closing quote fails to match, so there
// is no out-of-bounds range to crash on.
let escaped = NSRegularExpression.escapedPattern(for: attribute)
guard let regex = try? NSRegularExpression(pattern: "(?<![\\w-])\(escaped)=\"([^\"]*)\"") else {
return ""
}

return value
let range = NSRange(element.startIndex..., in: element)
guard let match = regex.firstMatch(in: element, range: range),
let valueRange = Range(match.range(at: 1), in: element)
else {
return ""
}
return String(element[valueRange])
}

/// Removes any trailing BR tags from the end of the specified string.
Expand All @@ -206,10 +207,9 @@ import Foundation
}

var content = string.trim()
let matches = RegEx.trailingBRTags.matches(in: content, options: .reportCompletion, range: NSRange(location: 0, length: content.count))
if let match = matches.first {
let index = content.index(content.startIndex, offsetBy: match.range.location)
content = String(content.prefix(upTo: index))
let matches = RegEx.trailingBRTags.matches(in: content, options: .reportCompletion, range: NSRange(location: 0, length: content.utf16.count))
if let match = matches.first, let matchRange = Range(match.range, in: content) {
content = String(content[..<matchRange.lowerBound])
}

return content
Expand Down Expand Up @@ -263,10 +263,10 @@ import Foundation
// For each video tag, check for controls attribute
for match in matches.reversed() {
let tag = mString.substring(with: match.range) as NSString
if !tag.contains("controls") {
// Add the controls attribute.
let range = NSRange(location: match.range.location, length: 6)
mString.replaceCharacters(in: range, with: "<video controls")
let controls = "\\scontrols(?![\\w-])"
if tag.range(of: controls, options: [.regularExpression, .caseInsensitive]).location == NSNotFound {
// Insert `controls` after the `<video` opening, preserving the tag's casing.
mString.insert(" controls", at: match.range.location + 6)
}
}

Expand Down
6 changes: 3 additions & 3 deletions Modules/Sources/WordPressSharedUI/RichContentFormatter+DisplayPipeline.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ extension RichContentFormatter {

let mImageStr = NSMutableString(string: imgElementStr)
mImageStr.replaceOccurrences(
of: srcImgURLStr,
with: modifiedURL.absoluteString,
of: "src=\"\(srcImgURLStr)\"",
with: "src=\"\(modifiedURL.absoluteString)\"",
options: .literal,
range: NSRange(location: 0, length: imgElementStr.count)
range: NSRange(location: 0, length: imgElementStr.utf16.count)
)

mContent.replaceCharacters(in: match.range, with: mImageStr as String)
Expand Down
Loading