Skip to content

Commit 3ef7f3f

Browse files
authored
Merge pull request github#11120 from atorralba/atorralba/swift/xxe-query-xmldocument-sinks
Swift: Adds XMLDocument sinks to the XXE query
2 parents 61149f2 + 52bd140 commit 3ef7f3f

File tree

3 files changed

+111
-2
lines changed

3 files changed

+111
-2
lines changed

swift/ql/lib/codeql/swift/security/XXE.qll

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ class XxeAdditionalTaintStep extends Unit {
2020
}
2121

2222
/** The XML argument of a `XMLParser` vulnerable to XXE. */
23-
private class DefaultXxeSink extends XxeSink {
24-
DefaultXxeSink() {
23+
private class XmlParserXxeSink extends XxeSink {
24+
XmlParserXxeSink() {
2525
this.asExpr() = any(Argument a | a.getApplyExpr() instanceof VulnerableParser).getExpr()
2626
}
2727
}
@@ -67,3 +67,26 @@ private class XmlParserRef extends Expr {
6767
private class XmlParserType extends NominalType {
6868
XmlParserType() { this.getFullName() = "XMLParser" }
6969
}
70+
71+
/** The XML argument of a `XMLDocument` vulnerable to XXE. */
72+
private class XmlDocumentXxeSink extends XxeSink {
73+
XmlDocumentXxeSink() { this.asExpr() = any(VulnerableXmlDocument d).getArgument(0).getExpr() }
74+
}
75+
76+
/** An `XMLDocument` that sets `nodeLoadExternalEntitiesAlways` in its options. */
77+
private class VulnerableXmlDocument extends ApplyExpr {
78+
VulnerableXmlDocument() {
79+
this.getStaticTarget().(ConstructorDecl).getEnclosingDecl().(NominalTypeDecl).getFullName() =
80+
"XMLDocument" and
81+
this.getArgument(1).getExpr().(ArrayExpr).getAnElement().(MemberRefExpr).getMember() instanceof
82+
NodeLoadExternalEntitiesAlways
83+
}
84+
}
85+
86+
/** The option `XMLNode.Options.nodeLoadExternalEntitiesAlways`. */
87+
private class NodeLoadExternalEntitiesAlways extends VarDecl {
88+
NodeLoadExternalEntitiesAlways() {
89+
this.getName() = "nodeLoadExternalEntitiesAlways" and
90+
this.getEnclosingDecl().(StructDecl).getFullName() = "XMLNode.Options"
91+
}
92+
}
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// --- stubs ---
2+
3+
class Data {
4+
init<S>(_ elements: S) {}
5+
}
6+
7+
struct URL {
8+
init?(string: String) {}
9+
}
10+
11+
extension String {
12+
init(contentsOf: URL) {
13+
let data = ""
14+
self.init(data)
15+
}
16+
}
17+
18+
class XMLNode {
19+
struct Options : OptionSet {
20+
let rawValue: Int
21+
static let nodeLoadExternalEntitiesAlways = XMLNode.Options(rawValue: 1 << 0)
22+
static let nodeLoadExternalEntitiesNever = XMLNode.Options(rawValue: 1 << 1)
23+
}
24+
}
25+
26+
class XMLElement {}
27+
28+
class XMLDocument {
29+
init(contentsOf: URL, options: XMLNode.Options = []) {}
30+
init(data: Data, options: XMLNode.Options = []) {}
31+
init(rootElement: XMLElement?) {}
32+
init(xmlString: String, options: XMLNode.Options = []) {}
33+
}
34+
35+
// --- tests ---
36+
37+
func testUrl() {
38+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
39+
let remoteUrl = URL(string: remoteString)!
40+
let _ = XMLDocument(contentsOf: remoteUrl, options: [.nodeLoadExternalEntitiesAlways]) // $ hasXXE=38
41+
}
42+
43+
func testUrlSafeImplicit() {
44+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
45+
let remoteUrl = URL(string: remoteString)!
46+
let _ = XMLDocument(contentsOf: remoteUrl, options: []) // NO XXE: document doesn't enable external entities
47+
}
48+
49+
func testUrlSafeExplicit() {
50+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
51+
let remoteUrl = URL(string: remoteString)!
52+
let _ = XMLDocument(contentsOf: remoteUrl, options: [.nodeLoadExternalEntitiesNever]) // NO XXE: document disables external entities
53+
}
54+
55+
func testData() {
56+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
57+
let remoteData = Data(remoteString)
58+
let _ = XMLDocument(data: remoteData, options: [.nodeLoadExternalEntitiesAlways]) // $ hasXXE=56
59+
}
60+
61+
func testDataSafeImplicit() {
62+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
63+
let remoteData = Data(remoteString)
64+
let _ = XMLDocument(data: remoteData, options: []) // NO XXE: document doesn't enable external entities
65+
}
66+
67+
func testDataSafeExplicit() {
68+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
69+
let remoteData = Data(remoteString)
70+
let _ = XMLDocument(data: remoteData, options: [.nodeLoadExternalEntitiesNever]) // NO XXE: document disables external entities
71+
}
72+
73+
func testString() {
74+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
75+
let _ = XMLDocument(xmlString: remoteString, options: [.nodeLoadExternalEntitiesAlways]) // $ hasXXE=74
76+
}
77+
78+
func testStringSafeImplicit() {
79+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
80+
let _ = XMLDocument(xmlString: remoteString, options: []) // NO XXE: document doesn't enable external entities
81+
}
82+
83+
func testStringSafeExplicit() {
84+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
85+
let _ = XMLDocument(xmlString: remoteString, options: [.nodeLoadExternalEntitiesNever]) // NO XXE: document disables external entities
86+
}

0 commit comments

Comments
 (0)