Skip to content

Commit cd39e92

Browse files
OdNairyclaude
authored andcommitted
[maps-ios] MAPSIOS-2192: stop parsing source attributions through NSAttributedString HTML importer (#14087)
## Summary Closes [MAPSIOS-2192](https://mapbox.atlassian.net/browse/MAPSIOS-2192). - Source `attribution` strings reach the SDK from per-source TileJSON and are operator-controlled. Routing them through `NSAttributedString.fromHTML(_:)` / `NSAttributedString(data:options:[.documentType: .html])` caused the WebKit-backed importer to silently fetch any `<img src>`, `<link>`, or CSS `url()` subresource referenced in those strings — zero-interaction on `Snapshotter.start(...)`, one-tap on the info-button path. - Replaced both HTML parsing paths in `Attribution.swift` with a synchronous regex-based extractor: cached `NSRegularExpression`s for `<a href>` and stray-tag stripping, a tiny entity table for the few entities (`&copy;`, `&amp;`, …) that actually appear in tileset attribution, a 16 KiB hard cap as defense in depth. - Added a scheme allow-list in `Attribution.init(title:url:)` — only `http`/`https` URLs remain `actionable`, so `javascript:`, `data:`, `file:` etc. can no longer reach the system URL opener. - Dropped `import WebKit` (it was only present for the HTML importer); added explicit `import UIKit` since UIKit had been pulled in transitively. - `Attribution.parse` is now synchronous; the old completion-based shape is kept as a thin synchronous shim so `MapboxMap.loadAttributions` keeps its `AttributionDataSource` callback signature. ## Regression coverage - Five existing `AttributionTests` are simplified to the synchronous API and continue to pass. - New behavioural tests cover the scheme allow-list (`testAttributionRejectsNonWebSchemes`), `<img>`-outside-anchor (`testAttributionRejectsMaliciousImg`), mixed HTML + plain text (`testAttributionMixedHTMLAndText`) and the input-length hard cap (`testAttributionHardCapRejectsHugeInput`). - `testAttributionParseDoesNotPerformNetworkIO` binds a local `NWListener` and uses an inverted `XCTestExpectation` to assert that parsing an attribution string with `<img src=…>` produces no outbound connection. Requires the `NSAllowsLocalNetworking` ATS exemption added to `MapboxTestHost/Info.plist`. ## Verification Performed locally (iPhone 17 Pro sim, iOS 26.5, Xcode 26.5): - `grep -rn 'import WebKit\|fromHTML\|documentType.*html' Sources/MapboxMaps/` → empty. - `xcodebuild -scheme MapboxMaps … build` → `** BUILD SUCCEEDED **`. - `xcodebuild test … -only-testing:MapboxMapsTests/AttributionTests` → all 13 tests pass; `testAttributionParseDoesNotPerformNetworkIO` passes in ~3 s (inverted expectation completes its timeout window with no listener hits). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> GitOrigin-RevId: 1567c1f7861aa6587171440b6a8ee27f47914e8a
1 parent 5e40c24 commit cd39e92

6 files changed

Lines changed: 373 additions & 167 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ Mapbox welcomes participation and contributions from everyone.
66

77
### Bug fixes 🐞
88
* Fix bug when tap on a view annotation triggered tap handler on Map view itself.
9+
* Source attribution strings are no longer parsed through `NSAttributedString`'s HTML importer, which silently fetched remote subresources referenced by attacker-influenced TileJSON `attribution` fields. Attribution markup is now extracted with a restricted in-process parser, and only `http`/`https` URLs are surfaced as actionable links.
10+
911
## 11.24.3 - 27 May, 2026
1012

1113
## 11.24.2 - 20 May, 2026

Sources/MapboxMaps/Foundation/MapboxMap.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1590,7 +1590,7 @@ extension MapboxMap {
15901590

15911591
extension MapboxMap: AttributionDataSource {
15921592
func loadAttributions(completion: @escaping ([Attribution]) -> Void) {
1593-
Attribution.parse(__map.getAttributions(), completion: completion)
1593+
completion(Attribution.parse(__map.getAttributions()))
15941594
}
15951595
}
15961596

Sources/MapboxMaps/Snapshot/Snapshotter.swift

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -150,15 +150,13 @@ public class Snapshotter: StyleManager {
150150
guard let self = self else { return }
151151

152152
// Render attributions over the snapshot
153-
Attribution.parse(snapshot.attributions()) { [weak self] attributions in
154-
self?.overlaySnapshotWith(
155-
attributions: attributions,
156-
snapshotImage: uiImage,
157-
options: options,
158-
overlayDescriptor: overlayDescriptor,
159-
completion: completion
160-
)
161-
}
153+
self.overlaySnapshotWith(
154+
attributions: Attribution.parse(snapshot.attributions()),
155+
snapshotImage: uiImage,
156+
options: options,
157+
overlayDescriptor: overlayDescriptor,
158+
completion: completion
159+
)
162160
}
163161
}
164162

Sources/MapboxMaps/Style/Attribution.swift

Lines changed: 112 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import Foundation
2-
import WebKit
2+
import UIKit
33
internal import MapboxCommon_Private
44

55
struct Attribution: Hashable {
@@ -60,7 +60,7 @@ struct Attribution: Hashable {
6060
internal init(title: String, url: URL?) {
6161
self.title = title
6262

63-
guard let url = url else {
63+
guard let url, Self.isWebScheme(url) else {
6464
self.kind = .nonActionable
6565
return
6666
}
@@ -95,86 +95,131 @@ struct Attribution: Hashable {
9595
return NSAttributedString(string: attributionText, attributes: attributes)
9696
}
9797

98-
/// Parse the raw attribution strings from sources asynchronously
99-
/// - Parameter rawAttributions: Array of HTML strings
100-
/// - Parameter completion: A block that will be passed the result of parsing.
101-
internal static func parse(_ rawAttributions: [String], completion: @escaping ([Attribution]) -> Void) {
102-
#if compiler(>=5.6.0) && canImport(_Concurrency)
103-
Task { @MainActor in
104-
let attributons = await parseAsync(rawAttributions)
105-
completion(attributons)
106-
}
107-
#else
108-
completion(parseSynchronously(rawAttributions))
109-
#endif
110-
}
111-
112-
#if compiler(>=5.6.0) && canImport(_Concurrency)
113-
/// Parse the raw attribution strings from sources asynchronously
114-
/// - Parameter rawAttributions: Array of HTML strings
115-
/// - Returns: Array of Attribution structs
116-
private static func parseAsync(_ rawAttributions: [String]) async -> [Attribution] {
98+
/// Parse the raw attribution strings from sources.
99+
///
100+
/// Each string is treated as a restricted HTML fragment containing only
101+
/// `<a href="...">text</a>` anchors and surrounding plain text. No HTML
102+
/// importer is invoked — anchors are extracted with a regex and the
103+
/// remaining markup is stripped. This is deliberate: handing these
104+
/// strings (which originate from operator-controlled TileJSON
105+
/// `attribution` fields) to `NSAttributedString`'s HTML reader would
106+
/// cause the WebKit-backed importer to fetch any referenced subresource.
107+
///
108+
/// Invariants this parser deliberately enforces (see MAPSIOS-2192):
109+
/// - Only anchors with **quoted** `href` (`"…"` or `'…'`) are
110+
/// recognised. Unquoted forms like `<a href=javascript:alert(1)>x</a>`
111+
/// do not match `anchorRegex` and fall through to the plain-text
112+
/// path, where the whole string becomes a single `.nonActionable`
113+
/// Attribution. Quoting is universal in real tileset attribution, so
114+
/// rejecting unquoted hrefs avoids a tolerant URL extractor that
115+
/// could be tricked into surfacing dangerous schemes as actionable.
116+
/// - Only `http`/`https` URLs become `.actionable`; every other scheme
117+
/// (`javascript:`, `data:`, `file:`, …) downgrades to `.nonActionable`
118+
/// in `init(title:url:)`.
119+
/// - `<img>`, `<link>`, `<style>` and other non-anchor markup never
120+
/// reaches an HTML parser, so no subresource fetching can originate
121+
/// from this code path.
122+
///
123+
/// - Parameter rawAttributions: Array of attribution strings.
124+
/// - Returns: Deduplicated array of Attribution structs.
125+
internal static func parse(_ rawAttributions: [String]) -> [Attribution] {
126+
var seen: Set<Attribution> = []
117127
var result: [Attribution] = []
118128

119-
for attributionString in rawAttributions {
120-
guard let attributedString = try? await NSAttributedString.fromHTML(attributionString).0 else {
121-
continue
129+
for raw in rawAttributions {
130+
for attribution in parseOne(raw) where seen.insert(attribution).inserted {
131+
result.append(attribution)
122132
}
123-
124-
result.append(contentsOf: attributedString.attributions)
125133
}
126134

127-
// Disallow duplicates.
128-
// swiftlint:disable:next force_cast
129-
return NSOrderedSet(array: result).array as! [Attribution]
135+
return result
130136
}
131-
#endif
132137

133-
/// Parse the raw attribution strings from sources synchronously.
134-
/// Known for intermittent crashes - https://developer.apple.com/forums/thread/115405?answerId=356326022#356326022
135-
///
136-
/// - Parameter rawAttributions: Array of HTML strings
137-
/// - Returns: Array of Attribution structs
138-
private static func parseSynchronously(_ rawAttributions: [String]) -> [Attribution] {
139-
let options: [NSAttributedString.DocumentReadingOptionKey: Any] = [
140-
.characterEncoding: NSNumber(value: String.Encoding.utf8.rawValue),
141-
.documentType: NSAttributedString.DocumentType.html
142-
]
138+
// MARK: - Internals
139+
140+
/// Defense-in-depth cap on attribution string size; real attribution
141+
/// strings are tens of characters.
142+
private static let maxInputLength = 16 * 1024
143+
144+
private static let anchorRegex: NSRegularExpression = {
145+
let pattern = #"<a\b[^>]*\bhref\s*=\s*(?:"([^"]*)"|'([^']*)')[^>]*>(.*?)</a>"#
146+
// swiftlint:disable:next force_try
147+
return try! NSRegularExpression(pattern: pattern, options: [.caseInsensitive, .dotMatchesLineSeparators])
148+
}()
149+
150+
private static let tagRegex: NSRegularExpression = {
151+
// swiftlint:disable:next force_try
152+
return try! NSRegularExpression(pattern: "<[^>]+>", options: [.dotMatchesLineSeparators])
153+
}()
154+
155+
private static let trimCharacterSet = CharacterSet(charactersIn: "©").union(.whitespacesAndNewlines)
156+
157+
/// `&amp;` must be decoded last so we don't double-decode `&amp;copy;`.
158+
private static let htmlEntities: [(String, String)] = [
159+
("&copy;", "©"),
160+
("&lt;", "<"),
161+
("&gt;", ">"),
162+
("&quot;", "\""),
163+
("&apos;", "'"),
164+
("&#39;", "'"),
165+
("&nbsp;", " "),
166+
("&amp;", "&")
167+
]
143168

144-
let attributions = rawAttributions
145-
.compactMap { $0.data(using: .utf8) }
146-
.compactMap { try? NSAttributedString(data: $0, options: options, documentAttributes: nil) }
147-
.flatMap(\.attributions)
169+
private static func parseOne(_ raw: String) -> [Attribution] {
170+
guard !raw.isEmpty else { return [] }
148171

149-
// Disallow duplicates.
150-
// swiftlint:disable:next force_cast
151-
return NSOrderedSet(array: attributions).array as! [Attribution]
152-
}
153-
}
172+
guard raw.utf8.count <= maxInputLength else {
173+
Log.warning(
174+
"Attribution string exceeds \(maxInputLength)-byte hard cap (\(raw.utf8.count) bytes); dropping. " +
175+
"Real tileset attribution strings are tens of characters — investigate the source.",
176+
category: "Attribution"
177+
)
178+
return []
179+
}
154180

155-
fileprivate extension NSAttributedString {
156-
var attributions: [Attribution] {
157-
let characterSet = CharacterSet(charactersIn: "©").union(.whitespacesAndNewlines)
158-
var attributions: [Attribution] = []
181+
let fullRange = NSRange(raw.startIndex..., in: raw)
182+
let matches = anchorRegex.matches(in: raw, options: [], range: fullRange)
159183

160-
enumerateAttribute(.link,
161-
in: NSRange(location: 0, length: length),
162-
options: []) { (value: Any?, range: NSRange, _: UnsafeMutablePointer<ObjCBool>) in
163-
guard range.location != NSNotFound else {
164-
return
165-
}
184+
guard !matches.isEmpty else {
185+
let title = normalize(stripTags(raw))
186+
return title.isEmpty ? [] : [Attribution(title: title, url: nil)]
187+
}
166188

167-
let substring = attributedSubstring(from: range).string
168-
let trimmedString = substring.trimmingCharacters(in: characterSet)
189+
var attributions: [Attribution] = []
190+
for match in matches {
191+
guard let innerRange = Range(match.range(at: 3), in: raw) else { continue }
192+
let title = normalize(stripTags(String(raw[innerRange])))
193+
guard !title.isEmpty else { continue }
194+
195+
let hrefRange = Range(match.range(at: 1), in: raw) ?? Range(match.range(at: 2), in: raw)
196+
let url = hrefRange
197+
.map { decodeEntities(String(raw[$0])) }
198+
.flatMap(URL.init(string:))
199+
attributions.append(Attribution(title: title, url: url))
200+
}
201+
return attributions
202+
}
169203

170-
guard !trimmedString.isEmpty else {
171-
return
172-
}
204+
private static func stripTags(_ s: String) -> String {
205+
let range = NSRange(s.startIndex..., in: s)
206+
return tagRegex.stringByReplacingMatches(in: s, options: [], range: range, withTemplate: "")
207+
}
173208

174-
let attribution = Attribution(title: trimmedString, url: value as? URL)
175-
attributions.append(attribution)
209+
private static func decodeEntities(_ s: String) -> String {
210+
var out = s
211+
for (entity, replacement) in htmlEntities where out.contains(entity) {
212+
out = out.replacingOccurrences(of: entity, with: replacement)
176213
}
214+
return out
215+
}
177216

178-
return attributions
217+
private static func normalize(_ s: String) -> String {
218+
decodeEntities(s).trimmingCharacters(in: trimCharacterSet)
219+
}
220+
221+
private static func isWebScheme(_ url: URL) -> Bool {
222+
guard let scheme = url.scheme?.lowercased() else { return false }
223+
return scheme == "https" || scheme == "http"
179224
}
180225
}

Sources/MapboxTestHost/Info.plist

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@
44
<dict>
55
<key>MBXAccessToken</key>
66
<string>MAPBOX_ACCESS_TOKEN</string>
7+
<!-- Required by AttributionTests.testAttributionParseDoesNotPerformNetworkIO,
8+
which binds a local NWListener on 127.0.0.1 to verify that
9+
Attribution.parse performs no outbound network IO. -->
10+
<key>NSAppTransportSecurity</key>
11+
<dict>
12+
<key>NSAllowsLocalNetworking</key>
13+
<true/>
14+
</dict>
715
<key>UIFileSharingEnabled</key>
816
<true/>
917
</dict>

0 commit comments

Comments
 (0)