Skip to content

Commit 0570610

Browse files
atorralbaatorralba
authored
Merge pull request github#11138 from atorralba/atorralba/swift/xxe-query-aexml-sinks
Swift: Add AEXML sinks to XXE query
2 parents f7b5a4d + 5791e8b commit 0570610

File tree

3 files changed

+277
-0
lines changed

3 files changed

+277
-0
lines changed

‎swift/ql/lib/codeql/swift/frameworks/AEXML.qll

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import swift
2+
3+
/** The creation of an `AEXMLParser`. */
4+
class AexmlParser extends ApplyExpr {
5+
AexmlParser() {
6+
this.getStaticTarget().(ConstructorDecl).getEnclosingDecl() instanceof AexmlParserDecl
7+
}
8+
}
9+
10+
/** The creation of an `AEXMLDocument`. */
11+
class AexmlDocument extends ApplyExpr {
12+
AexmlDocument() {
13+
this.getStaticTarget().(ConstructorDecl).getEnclosingDecl() instanceof AexmlDocumentDecl
14+
}
15+
}
16+
17+
/** A call to `AEXMLDocument.loadXML(_:)`. */
18+
class AexmlDocumentLoadXml extends MethodApplyExpr {
19+
AexmlDocumentLoadXml() {
20+
exists(MethodDecl f |
21+
this.getStaticTarget() = f and
22+
f.hasName("loadXML(_:)") and
23+
f.getEnclosingDecl() instanceof AexmlDocumentDecl
24+
)
25+
}
26+
}
27+
28+
/** The class `AEXMLParser`. */
29+
class AexmlParserDecl extends ClassDecl {
30+
AexmlParserDecl() { this.getFullName() = "AEXMLParser" }
31+
}
32+
33+
/** The class `AEXMLDocument`. */
34+
class AexmlDocumentDecl extends ClassDecl {
35+
AexmlDocumentDecl() { this.getFullName() = "AEXMLDocument" }
36+
}
37+
38+
/** A reference to the field `AEXMLOptions.ParserSettings.shouldResolveExternalEntities`. */
39+
class AexmlShouldResolveExternalEntities extends MemberRefExpr {
40+
AexmlShouldResolveExternalEntities() {
41+
exists(FieldDecl f | this.getMember() = f |
42+
f.getName() = "shouldResolveExternalEntities" and
43+
f.getEnclosingDecl().(NominalTypeDecl).getType() instanceof AexmlOptionsParserSettingsType
44+
)
45+
}
46+
}
47+
48+
/** The type `AEXMLOptions`. */
49+
class AexmlOptionsType extends StructType {
50+
AexmlOptionsType() { this.getFullName() = "AEXMLOptions" }
51+
}
52+
53+
/** The type `AEXMLOptions.ParserSettings`. */
54+
class AexmlOptionsParserSettingsType extends StructType {
55+
AexmlOptionsParserSettingsType() { this.getFullName() = "AEXMLOptions.ParserSettings" }
56+
}

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

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import swift
44
private import codeql.swift.dataflow.DataFlow
5+
private import codeql.swift.frameworks.AEXML
56

67
/** A data flow sink for XML external entities (XXE) vulnerabilities. */
78
abstract class XxeSink extends DataFlow::Node { }
@@ -90,3 +91,75 @@ private class NodeLoadExternalEntitiesAlways extends VarDecl {
9091
this.getEnclosingDecl().(StructDecl).getFullName() = "XMLNode.Options"
9192
}
9293
}
94+
95+
/** The XML argument of an `AEXMLDocument` vulnerable to XXE. */
96+
private class AexmlDocumentSink extends XxeSink {
97+
AexmlDocumentSink() {
98+
// `AEXMLDocument` initialized with vulnerable options.
99+
exists(ApplyExpr call | this.asExpr() = call.getArgument(0).getExpr() |
100+
call.(VulnerableAexmlDocument)
101+
.getStaticTarget()
102+
.hasName(["init(xml:options:)", "init(xml:encoding:options:)"])
103+
or
104+
// `loadXML` called on a vulnerable AEXMLDocument.
105+
DataFlow::localExprFlow(any(VulnerableAexmlDocument v),
106+
call.(AexmlDocumentLoadXml).getQualifier())
107+
)
108+
}
109+
}
110+
111+
/** The XML argument of an `AEXMLParser` initialized with an `AEXMLDocument` vulnerable to XXE. */
112+
private class AexmlParserSink extends XxeSink {
113+
AexmlParserSink() {
114+
exists(AexmlParser parser | this.asExpr() = parser.getArgument(1).getExpr() |
115+
DataFlow::localExprFlow(any(VulnerableAexmlDocument v), parser.getArgument(0).getExpr())
116+
)
117+
}
118+
}
119+
120+
/** The creation of an `AEXMLDocument` that receives a vulnerable `AEXMLOptions` argument. */
121+
private class VulnerableAexmlDocument extends AexmlDocument {
122+
VulnerableAexmlDocument() {
123+
exists(AexmlOptions optionsArgument, VulnerableAexmlOptions vulnOpts |
124+
this.getAnArgument().getExpr() = optionsArgument and
125+
DataFlow::localExprFlow(vulnOpts, optionsArgument)
126+
)
127+
}
128+
}
129+
130+
/**
131+
* An `AEXMLOptions` object which contains a `parserSettings` with `shouldResolveExternalEntities`
132+
* set to `true`.
133+
*/
134+
private class VulnerableAexmlOptions extends AexmlOptions {
135+
VulnerableAexmlOptions() {
136+
exists(
137+
AexmlParserSettings parserSettings, AexmlShouldResolveExternalEntities sree, AssignExpr a
138+
|
139+
a.getSource() = any(BooleanLiteralExpr b | b.getValue() = true) and
140+
a.getDest() = sree and
141+
sree.(MemberRefExpr).getBase() = parserSettings and
142+
parserSettings.(MemberRefExpr).getBase() = this
143+
)
144+
}
145+
}
146+
147+
/** An expression of type `AEXMLOptions.ParserSettings`. */
148+
private class AexmlParserSettings extends Expr {
149+
pragma[inline]
150+
AexmlParserSettings() {
151+
this.getType() instanceof AexmlOptionsParserSettingsType or
152+
this.getType() = any(OptionalType t | t.getBaseType() instanceof AexmlOptionsParserSettingsType) or
153+
this.getType() = any(LValueType t | t.getObjectType() instanceof AexmlOptionsParserSettingsType)
154+
}
155+
}
156+
157+
/** An expression of type `AEXMLOptions`. */
158+
private class AexmlOptions extends Expr {
159+
pragma[inline]
160+
AexmlOptions() {
161+
this.getType() instanceof AexmlOptionsType or
162+
this.getType() = any(OptionalType t | t.getBaseType() instanceof AexmlOptionsType) or
163+
this.getType() = any(LValueType t | t.getObjectType() instanceof AexmlOptionsType)
164+
}
165+
}

‎swift/ql/test/query-tests/Security/CWE-611/testAEXMLDocumentXXE.swift

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
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+
struct Encoding: Hashable {
13+
let rawValue: UInt
14+
static let utf8 = String.Encoding(rawValue: 1)
15+
}
16+
17+
init(contentsOf: URL) {
18+
let data = ""
19+
self.init(data)
20+
}
21+
}
22+
23+
class AEXMLElement {}
24+
25+
struct AEXMLOptions {
26+
var parserSettings = ParserSettings()
27+
28+
struct ParserSettings {
29+
public var shouldResolveExternalEntities = false
30+
}
31+
}
32+
33+
class AEXMLDocument {
34+
init(root: AEXMLElement? = nil, options: AEXMLOptions) {}
35+
init(xml: Data, options: AEXMLOptions = AEXMLOptions()) {}
36+
init(xml: String, encoding: String.Encoding, options: AEXMLOptions) {}
37+
func loadXML(_: Data) {}
38+
}
39+
40+
class AEXMLParser {
41+
init(document: AEXMLDocument, data: Data) {}
42+
}
43+
44+
// --- tests ---
45+
46+
func testString() {
47+
var options = AEXMLOptions()
48+
options.parserSettings.shouldResolveExternalEntities = true
49+
50+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
51+
let _ = AEXMLDocument(xml: remoteString, encoding: String.Encoding.utf8, options: options) // $ hasXXE=50
52+
}
53+
54+
func testStringSafeImplicit() {
55+
var options = AEXMLOptions()
56+
57+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
58+
let _ = AEXMLDocument(xml: remoteString, encoding: String.Encoding.utf8, options: options) // NO XXE
59+
}
60+
61+
func testStringSafeExplicit() {
62+
var options = AEXMLOptions()
63+
options.parserSettings.shouldResolveExternalEntities = false
64+
65+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
66+
let _ = AEXMLDocument(xml: remoteString, encoding: String.Encoding.utf8, options: options) // NO XXE
67+
}
68+
69+
func testData() {
70+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
71+
let remoteData = Data(remoteString)
72+
var options = AEXMLOptions()
73+
options.parserSettings.shouldResolveExternalEntities = true
74+
let _ = AEXMLDocument(xml: remoteData, options: options) // $ hasXXE=70
75+
}
76+
77+
func testDataSafeImplicit() {
78+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
79+
let remoteData = Data(remoteString)
80+
var options = AEXMLOptions()
81+
let _ = AEXMLDocument(xml: remoteData, options: options) // NO XXE
82+
}
83+
84+
func testDataSafeExplicit() {
85+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
86+
let remoteData = Data(remoteString)
87+
var options = AEXMLOptions()
88+
options.parserSettings.shouldResolveExternalEntities = false
89+
let _ = AEXMLDocument(xml: remoteData, options: options) // NO XXE
90+
}
91+
92+
func testDataLoadXml() {
93+
var options = AEXMLOptions()
94+
options.parserSettings.shouldResolveExternalEntities = true
95+
let doc = AEXMLDocument(root: nil, options: options)
96+
97+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
98+
let remoteData = Data(remoteString)
99+
doc.loadXML(remoteData) // $ hasXXE=97
100+
}
101+
102+
func testDataLoadXmlSafeImplicit() {
103+
var options = AEXMLOptions()
104+
let doc = AEXMLDocument(root: nil, options: options)
105+
106+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
107+
let remoteData = Data(remoteString)
108+
doc.loadXML(remoteData) // NO XXE
109+
}
110+
111+
func testDataLoadXmlSafeExplicit() {
112+
var options = AEXMLOptions()
113+
options.parserSettings.shouldResolveExternalEntities = false
114+
let doc = AEXMLDocument(root: nil, options: options)
115+
116+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
117+
let remoteData = Data(remoteString)
118+
doc.loadXML(remoteData) // NO XXE
119+
}
120+
121+
func testParser() {
122+
var options = AEXMLOptions()
123+
options.parserSettings.shouldResolveExternalEntities = true
124+
let doc = AEXMLDocument(root: nil, options: options)
125+
126+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
127+
let remoteData = Data(remoteString)
128+
let _ = AEXMLParser(document: doc, data: remoteData) // $ hasXXE=126
129+
}
130+
131+
func testParserSafeImplicit() {
132+
var options = AEXMLOptions()
133+
let doc = AEXMLDocument(root: nil, options: options)
134+
135+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
136+
let remoteData = Data(remoteString)
137+
let _ = AEXMLParser(document: doc, data: remoteData) // NO XXE
138+
}
139+
140+
func testParserSafeExplicit() {
141+
var options = AEXMLOptions()
142+
options.parserSettings.shouldResolveExternalEntities = false
143+
let doc = AEXMLDocument(root: nil, options: options)
144+
145+
let remoteString = String(contentsOf: URL(string: "http://example.com/")!)
146+
let remoteData = Data(remoteString)
147+
let _ = AEXMLParser(document: doc, data: remoteData) // NO XXE
148+
}

0 commit comments

Comments
 (0)