Skip to content

Commit 1267012

Browse files
feat(security): disable external entity loading in SVG parser
- Initializes `XMLDocument` with `.nodeLoadExternalEntitiesNever` in `SVGParser.swift`. - Prevents XML External Entity (XXE) attacks when parsing untrusted SVG files. - Hardens the codebase against local file inclusion and SSRF vulnerabilities via malicious SVGs.
1 parent 4692a0f commit 1267012

3 files changed

Lines changed: 8 additions & 1 deletion

File tree

.jules/sentinel.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
## 2024-05-22 - [XXE Protection in SVG Parser]
2+
**Vulnerability:** The `XMLDocument` initializer in `SVGParser.swift` used empty options `[]`, which potentially allows XML External Entity (XXE) attacks if the underlying parser defaults are permissive.
3+
**Learning:** Even when using higher-level abstractions like `XMLDocument`, one must explicitly disable dangerous features like external entity loading when processing untrusted input.
4+
**Prevention:** Always use `.nodeLoadExternalEntitiesNever` (or equivalent flags in other parsers) when parsing XML/SVG data from external sources.

Sources/SVGKit/SVGParser.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,9 @@ public final class SVGParser: @unchecked Sendable { // swiftlint:disable:this ty
327327
data
328328
}
329329

330-
let document = try XMLDocument(data: svgData, options: [])
330+
// Disable external entity loading to prevent XXE attacks
331+
let options: XMLNode.Options = [.nodeLoadExternalEntitiesNever]
332+
let document = try XMLDocument(data: svgData, options: options)
331333
guard let root = document.rootElement(), elementName(root) == "svg" else {
332334
throw SVGParserError.invalidSVGRoot
333335
}

mise.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)