Skip to content

Commit 6cfa89e

Browse files
atorralbaatorralba
authored
Merge pull request #11165 from atorralba/atorralba/swift/xxe-query-libxml2-sinks
Swift: Add libxml2 sinks to the XXE query
2 parents 28c32fc + 16a7685 commit 6cfa89e

File tree

4 files changed

+195
-0
lines changed

4 files changed

+195
-0
lines changed

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import swift
2+
3+
/**
4+
* A call to a `libxml2` function that parses XML.
5+
*/
6+
class Libxml2ParseCall extends ApplyExpr {
7+
int xmlArg;
8+
int optionsArg;
9+
10+
Libxml2ParseCall() {
11+
exists(string fname | this.getStaticTarget().getName() = fname |
12+
fname = "xmlCtxtUseOptions(_:_:)" and xmlArg = 0 and optionsArg = 1
13+
or
14+
fname = "xmlReadFile(_:_:_:)" and xmlArg = 0 and optionsArg = 2
15+
or
16+
fname = ["xmlReadDoc(_:_:_:_:)", "xmlReadFd(_:_:_:_:)"] and
17+
xmlArg = 0 and
18+
optionsArg = 3
19+
or
20+
fname = ["xmlCtxtReadFile(_:_:_:_:)", "xmlParseInNodeContext(_:_:_:_:_:)"] and
21+
xmlArg = 1 and
22+
optionsArg = 3
23+
or
24+
fname = ["xmlCtxtReadDoc(_:_:_:_:_:)", "xmlCtxtReadFd(_:_:_:_:_:)"] and
25+
xmlArg = 1 and
26+
optionsArg = 4
27+
or
28+
fname = "xmlReadMemory(_:_:_:_:_:)" and xmlArg = 0 and optionsArg = 4
29+
or
30+
fname = "xmlCtxtReadMemory(_:_:_:_:_:_:)" and xmlArg = 1 and optionsArg = 5
31+
or
32+
fname = "xmlReadIO(_:_:_:_:_:_:)" and xmlArg = 0 and optionsArg = 5
33+
or
34+
fname = "xmlCtxtReadIO(_:_:_:_:_:_:_:)" and xmlArg = 1 and optionsArg = 6
35+
)
36+
}
37+
38+
/**
39+
* Gets the argument that receives the XML raw data.
40+
*/
41+
Expr getXml() { result = this.getArgument(xmlArg).getExpr() }
42+
43+
/**
44+
* Gets the argument that specifies `xmlParserOption`s.
45+
*/
46+
Expr getOptions() { result = this.getArgument(optionsArg).getExpr() }
47+
}
48+
49+
/**
50+
* An `xmlParserOption` for `libxml2` that is considered unsafe.
51+
*/
52+
class Libxml2BadOption extends ConcreteVarDecl {
53+
Libxml2BadOption() { this.getName() = ["XML_PARSE_NOENT", "XML_PARSE_DTDLOAD"] }
54+
}

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import swift
44
private import codeql.swift.dataflow.DataFlow
55
private import codeql.swift.frameworks.AEXML
6+
private import codeql.swift.frameworks.Libxml2
67

78
/** A data flow sink for XML external entities (XXE) vulnerabilities. */
89
abstract class XxeSink extends DataFlow::Node { }
@@ -163,3 +164,40 @@ private class AexmlOptions extends Expr {
163164
this.getType() = any(LValueType t | t.getObjectType() instanceof AexmlOptionsType)
164165
}
165166
}
167+
168+
/** The XML argument of a `libxml2` parsing call vulnerable to XXE. */
169+
private class Libxml2XxeSink extends XxeSink {
170+
Libxml2XxeSink() {
171+
exists(Libxml2ParseCall c, Libxml2BadOption opt |
172+
this.asExpr() = c.getXml() and
173+
lib2xmlOptionLocalTaintStep*(DataFlow::exprNode(opt.getAnAccess()),
174+
DataFlow::exprNode(c.getOptions()))
175+
)
176+
}
177+
}
178+
179+
/**
180+
* Holds if taint can flow from `source` to `sink` in one local step,
181+
* including bitwise operations, accesses to `.rawValue`, and casts to `Int32`.
182+
*/
183+
private predicate lib2xmlOptionLocalTaintStep(DataFlow::Node source, DataFlow::Node sink) {
184+
DataFlow::localFlowStep(source, sink)
185+
or
186+
source.asExpr() = sink.asExpr().(BitwiseOperation).getAnOperand()
187+
or
188+
exists(MemberRefExpr rawValue | rawValue.getMember().(VarDecl).getName() = "rawValue" |
189+
source.asExpr() = rawValue.getBase() and sink.asExpr() = rawValue
190+
)
191+
or
192+
exists(ApplyExpr int32Init |
193+
int32Init
194+
.getStaticTarget()
195+
.(ConstructorDecl)
196+
.getEnclosingDecl()
197+
.(ExtensionDecl)
198+
.getExtendedTypeDecl()
199+
.getName() = "SignedInteger"
200+
|
201+
source.asExpr() = int32Init.getAnArgument().getExpr() and sink.asExpr() = int32Init
202+
)
203+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,14 @@
11
import swift
2+
import codeql.swift.dataflow.FlowSources
23
import codeql.swift.security.XXEQuery
34
import TestUtilities.InlineExpectationsTest
45

6+
class TestRemoteSource extends RemoteFlowSource {
7+
TestRemoteSource() { this.asExpr().(ApplyExpr).getStaticTarget().getName().matches("source%") }
8+
9+
override string getSourceType() { result = "Test source" }
10+
}
11+
512
class XxeTest extends InlineExpectationsTest {
613
XxeTest() { this = "XxeTest" }
714

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

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
// --- stubs ---
2+
3+
class Data {
4+
init<S>(_ elements: S) {}
5+
func copyBytes(to: UnsafeMutablePointer<UInt8>, count: Int) {}
6+
}
7+
8+
struct URL {
9+
init?(string: String) {}
10+
}
11+
12+
extension String {
13+
init(contentsOf: URL) {
14+
let data = ""
15+
self.init(data)
16+
}
17+
}
18+
19+
struct xmlParserOption : Hashable {
20+
let rawValue: UInt32 = 0
21+
}
22+
23+
var XML_PARSE_NOENT: xmlParserOption { get { return xmlParserOption() } }
24+
var XML_PARSE_DTDLOAD: xmlParserOption { get { return xmlParserOption() } }
25+
26+
typealias xmlChar = UInt8
27+
typealias xmlDocPtr = UnsafeMutablePointer<xmlDoc>
28+
typealias xmlNodePtr = UnsafeMutablePointer<xmlNode>
29+
typealias xmlParserCtxtPtr = UnsafeMutablePointer<xmlParserCtxt>
30+
struct xmlDoc {}
31+
struct xmlNode {}
32+
struct xmlParserCtxt {}
33+
struct xmlParserErrors {}
34+
struct xmlInputReadCallback {}
35+
struct xmlInputCloseCallback {}
36+
37+
func xmlCtxtUseOptions(_ ctxt: xmlParserCtxtPtr!, _ options: Int32) -> Int32 { return 0 }
38+
func xmlReadDoc(_ cur: UnsafePointer<xmlChar>!, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
39+
func xmlReadFile(_ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
40+
func xmlReadMemory(_ buffer: UnsafePointer<CChar>!, _ size: Int32, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
41+
func xmlReadFd(_ fd: Int32, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
42+
func xmlReadIO(_ ioread: xmlInputReadCallback!, _ ioclose: xmlInputCloseCallback!, _ ioctx: UnsafeMutableRawPointer!, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
43+
func xmlCtxtReadDoc(_ ctxt: xmlParserCtxtPtr!, _ cur: UnsafePointer<xmlChar>!, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
44+
func xmlCtxtReadFile(_ ctxt: xmlParserCtxtPtr!, _ filename: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
45+
func xmlParseInNodeContext(_ node: xmlNodePtr!, _ data: UnsafePointer<CChar>!, _ datalen: Int32, _ options: Int32, _ lst: UnsafeMutablePointer<xmlNodePtr?>!) -> xmlParserErrors { return xmlParserErrors() }
46+
func xmlCtxtReadMemory(_ ctxt: xmlParserCtxtPtr!, _ buffer: UnsafePointer<CChar>!, _ size: Int32, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
47+
func xmlCtxtReadFd(_ ctxt: xmlParserCtxtPtr!, _ fd: Int32, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
48+
func xmlCtxtReadIO(_ ctxt: xmlParserCtxtPtr!, _ ioread: xmlInputReadCallback!, _ ioclose: xmlInputCloseCallback!, _ ioctx: UnsafeMutableRawPointer!, _ URL: UnsafePointer<CChar>!, _ encoding: UnsafePointer<CChar>!, _ options: Int32) -> xmlDocPtr! { return xmlDocPtr.allocate(capacity: 0) }
49+
50+
// --- tests ---
51+
52+
func sourcePtr() -> UnsafeMutablePointer<UInt8> { return UnsafeMutablePointer<UInt8>.allocate(capacity: 0) }
53+
func sourceCharPtr() -> UnsafeMutablePointer<CChar> { return UnsafeMutablePointer<CChar>.allocate(capacity: 0) }
54+
55+
func test() {
56+
let remotePtr = sourcePtr()
57+
let remoteCharPtr = sourceCharPtr()
58+
let _ = xmlReadFile(remoteCharPtr, nil, 0) // NO XXE: external entities not enabled
59+
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=57
60+
let _ = xmlReadFile(remoteCharPtr, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
61+
let _ = xmlReadDoc(remotePtr, nil, nil, 0) // NO XXE: external entities not enabled
62+
let _ = xmlReadDoc(remotePtr, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=56
63+
let _ = xmlReadDoc(remotePtr, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=56
64+
let _ = xmlCtxtReadFile(nil, remoteCharPtr, nil, 0) // NO XXE: external entities not enabled
65+
let _ = xmlCtxtReadFile(nil, remoteCharPtr, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=57
66+
let _ = xmlCtxtReadFile(nil, remoteCharPtr, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
67+
let _ = xmlParseInNodeContext(nil, remoteCharPtr, -1, 0, nil) // NO XXE: external entities not enabled
68+
let _ = xmlParseInNodeContext(nil, remoteCharPtr, -1, Int32(XML_PARSE_DTDLOAD.rawValue), nil) // $ hasXXE=57
69+
let _ = xmlParseInNodeContext(nil, remoteCharPtr, -1, Int32(XML_PARSE_NOENT.rawValue), nil) // $ hasXXE=57
70+
let _ = xmlCtxtReadDoc(nil, remotePtr, nil, nil, 0) // NO XXE: external entities not enabled
71+
let _ = xmlCtxtReadDoc(nil, remotePtr, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=56
72+
let _ = xmlCtxtReadDoc(nil, remotePtr, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=56
73+
let _ = xmlReadMemory(remoteCharPtr, -1, nil, nil, 0) // NO XXE: external entities not enabled
74+
let _ = xmlReadMemory(remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=57
75+
let _ = xmlReadMemory(remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
76+
let _ = xmlCtxtReadMemory(nil, remoteCharPtr, -1, nil, nil, 0) // NO XXE: external entities not enabled
77+
let _ = xmlCtxtReadMemory(nil, remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_NOENT.rawValue)) // $ hasXXE=57
78+
let _ = xmlCtxtReadMemory(nil, remoteCharPtr, -1, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue)) // $ hasXXE=57
79+
// TODO: We would need to model taint around `xmlParserCtxtPtr`, file descriptors, and `xmlInputReadCallback`
80+
// to be able to alert on these methods. Not doing it for now because of the effort required vs the expected gain.
81+
let _ = xmlCtxtUseOptions(nil, 0)
82+
let _ = xmlCtxtUseOptions(nil, Int32(XML_PARSE_NOENT.rawValue))
83+
let _ = xmlCtxtUseOptions(nil, Int32(XML_PARSE_DTDLOAD.rawValue))
84+
let _ = xmlReadFd(0, nil, nil, 0)
85+
let _ = xmlReadFd(0, nil, nil, Int32(XML_PARSE_NOENT.rawValue))
86+
let _ = xmlReadFd(0, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue))
87+
let _ = xmlCtxtReadFd(nil, 0, nil, nil, 0)
88+
let _ = xmlCtxtReadFd(nil, 0, nil, nil, Int32(XML_PARSE_NOENT.rawValue))
89+
let _ = xmlCtxtReadFd(nil, 0, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue))
90+
let _ = xmlReadIO(nil, nil, nil, nil, nil, 0)
91+
let _ = xmlReadIO(nil, nil, nil, nil, nil, Int32(XML_PARSE_NOENT.rawValue))
92+
let _ = xmlReadIO(nil, nil, nil, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue))
93+
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, 0)
94+
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, Int32(XML_PARSE_NOENT.rawValue))
95+
let _ = xmlCtxtReadIO(nil, nil, nil, nil, nil, nil, Int32(XML_PARSE_DTDLOAD.rawValue))
96+
}

0 commit comments

Comments
 (0)