Skip to content

Commit eb4d30d

Browse files
Copilotgeoffw0
andcommitted
Add XXE query for Rust (CWE-611)
Co-authored-by: geoffw0 <40627776+geoffw0@users.noreply.github.com>
1 parent 65f7463 commit eb4d30d

File tree

11 files changed

+486
-0
lines changed

11 files changed

+486
-0
lines changed
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
/**
2+
* Provides classes and predicates to reason about XML external entity (XXE)
3+
* vulnerabilities.
4+
*/
5+
6+
import rust
7+
private import codeql.rust.dataflow.DataFlow
8+
private import codeql.rust.dataflow.FlowSink
9+
private import codeql.rust.Concepts
10+
11+
/**
12+
* Provides default sources, sinks and barriers for detecting XML external
13+
* entity (XXE) vulnerabilities, as well as extension points for adding your
14+
* own.
15+
*/
16+
module Xxe {
17+
/**
18+
* A data flow source for XXE vulnerabilities.
19+
*/
20+
abstract class Source extends DataFlow::Node { }
21+
22+
/**
23+
* A data flow sink for XXE vulnerabilities.
24+
*/
25+
abstract class Sink extends QuerySink::Range {
26+
override string getSinkType() { result = "Xxe" }
27+
}
28+
29+
/**
30+
* A barrier for XXE vulnerabilities.
31+
*/
32+
abstract class Barrier extends DataFlow::Node { }
33+
34+
/**
35+
* An active threat-model source, considered as a flow source.
36+
*/
37+
private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { }
38+
39+
/**
40+
* A libxml2 XML parsing call with unsafe parser options, considered as a
41+
* flow sink.
42+
*/
43+
private class Libxml2XxeSink extends Sink {
44+
Libxml2XxeSink() {
45+
exists(Call call, int xmlArg, int optionsArg |
46+
libxml2ParseCall(call, xmlArg, optionsArg) and
47+
this.asExpr() = call.getPositionalArgument(xmlArg) and
48+
hasXxeOption(call.getPositionalArgument(optionsArg))
49+
)
50+
}
51+
}
52+
}
53+
54+
/**
55+
* Holds if `call` is a call to a `libxml2` XML parsing function, where
56+
* `xmlArg` is the index of the XML content argument and `optionsArg` is the
57+
* index of the parser options argument.
58+
*/
59+
private predicate libxml2ParseCall(Call call, int xmlArg, int optionsArg) {
60+
exists(string fname | call.getStaticTarget().getName().getText() = fname |
61+
fname = "xmlCtxtUseOptions" and xmlArg = 0 and optionsArg = 1
62+
or
63+
fname = "xmlReadFile" and xmlArg = 0 and optionsArg = 2
64+
or
65+
fname = ["xmlReadDoc", "xmlReadFd"] and xmlArg = 0 and optionsArg = 3
66+
or
67+
fname = ["xmlCtxtReadFile", "xmlParseInNodeContext"] and xmlArg = 1 and optionsArg = 3
68+
or
69+
fname = ["xmlCtxtReadDoc", "xmlCtxtReadFd"] and xmlArg = 1 and optionsArg = 4
70+
or
71+
fname = "xmlReadMemory" and xmlArg = 0 and optionsArg = 4
72+
or
73+
fname = "xmlCtxtReadMemory" and xmlArg = 1 and optionsArg = 5
74+
or
75+
fname = "xmlReadIO" and xmlArg = 0 and optionsArg = 5
76+
or
77+
fname = "xmlCtxtReadIO" and xmlArg = 1 and optionsArg = 6
78+
)
79+
}
80+
81+
/**
82+
* Holds if `e` is an expression that includes an unsafe `xmlParserOption`,
83+
* specifically `XML_PARSE_NOENT` (value 2, enables entity substitution) or
84+
* `XML_PARSE_DTDLOAD` (value 4, loads external DTD subsets).
85+
*/
86+
private predicate hasXxeOption(Expr e) {
87+
// Named constant XML_PARSE_NOENT or XML_PARSE_DTDLOAD
88+
e.(PathExpr).getPath().getText() = ["XML_PARSE_NOENT", "XML_PARSE_DTDLOAD"]
89+
or
90+
// Integer literal with XML_PARSE_NOENT (bit 1) or XML_PARSE_DTDLOAD (bit 2) set
91+
exists(int v |
92+
v = e.(IntegerLiteralExpr).getTextValue().regexpCapture("^([0-9]+).*$", 1).toInt()
93+
|
94+
v.bitAnd(6) != 0 // 6 = 2 | 4 = XML_PARSE_NOENT | XML_PARSE_DTDLOAD
95+
)
96+
or
97+
// Bitwise OR expression
98+
hasXxeOption(e.(BinaryExpr).getLhs())
99+
or
100+
hasXxeOption(e.(BinaryExpr).getRhs())
101+
or
102+
// Cast expression (e.g., `XML_PARSE_NOENT as i32`)
103+
hasXxeOption(e.(CastExpr).getExpr())
104+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `rust/xxe`, to detect XML external entity (XXE) vulnerabilities in Rust code that uses the `libxml` crate (bindings to C's `libxml2`). The query flags calls to `libxml2` parsing functions with unsafe options (`XML_PARSE_NOENT` or `XML_PARSE_DTDLOAD`) when the XML input comes from a user-controlled source.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Parsing XML input with external entity (XXE) expansion enabled while the input
9+
is controlled by a user can lead to a variety of attacks. An attacker who
10+
controls the XML input may be able to use an XML external entity declaration
11+
to read the contents of arbitrary files from the server's file system, perform
12+
server-side request forgery (SSRF), or perform denial-of-service attacks.
13+
</p>
14+
<p>
15+
The Rust <code>libxml</code> crate (bindings to C's <code>libxml2</code>
16+
library) exposes several XML parsing functions that accept a parser options
17+
argument. The options <code>XML_PARSE_NOENT</code> and
18+
<code>XML_PARSE_DTDLOAD</code> enable external entity expansion and loading of
19+
external DTD subsets, respectively. Enabling these options when parsing
20+
user-controlled XML is dangerous.
21+
</p>
22+
</overview>
23+
24+
<recommendation>
25+
<p>
26+
Do not enable <code>XML_PARSE_NOENT</code> or <code>XML_PARSE_DTDLOAD</code>
27+
when parsing user-controlled XML. Parse XML with safe options (for example,
28+
using <code>0</code> as the options argument) to disable external entity
29+
expansion.
30+
</p>
31+
</recommendation>
32+
33+
<example>
34+
<p>
35+
In the following example, the program reads an XML document supplied by the
36+
user and parses it with external entity expansion enabled:
37+
</p>
38+
<sample src="examples/XxeBad.rs"/>
39+
<p>
40+
The following example shows a corrected version that parses with safe options:
41+
</p>
42+
<sample src="examples/XxeGood.rs"/>
43+
</example>
44+
45+
<references>
46+
<li>OWASP: <a href="https://owasp.org/www-community/vulnerabilities/XML_External_Entity_(XXE)_Processing">XML External Entity (XXE) Processing</a>.</li>
47+
<li>CWE: <a href="https://cwe.mitre.org/data/definitions/611.html">CWE-611: Improper Restriction of XML External Entity Reference</a>.</li>
48+
</references>
49+
50+
</qhelp>
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/**
2+
* @name XML external entity expansion
3+
* @description Parsing user-controlled XML with external entity expansion
4+
* enabled may lead to disclosure of confidential data or
5+
* server-side request forgery.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @security-severity 9.1
9+
* @precision high
10+
* @id rust/xxe
11+
* @tags security
12+
* external/cwe/cwe-611
13+
* external/cwe/cwe-776
14+
* external/cwe/cwe-827
15+
*/
16+
17+
import rust
18+
import codeql.rust.dataflow.DataFlow
19+
import codeql.rust.dataflow.TaintTracking
20+
import codeql.rust.security.XxeExtensions
21+
22+
/**
23+
* A taint configuration for user-controlled data reaching an XML parser with
24+
* external entity expansion enabled.
25+
*/
26+
module XxeConfig implements DataFlow::ConfigSig {
27+
import Xxe
28+
29+
predicate isSource(DataFlow::Node node) { node instanceof Source }
30+
31+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
32+
33+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
34+
35+
predicate observeDiffInformedIncrementalMode() { any() }
36+
}
37+
38+
module XxeFlow = TaintTracking::Global<XxeConfig>;
39+
40+
import XxeFlow::PathGraph
41+
42+
from XxeFlow::PathNode sourceNode, XxeFlow::PathNode sinkNode
43+
where XxeFlow::flowPath(sourceNode, sinkNode)
44+
select sinkNode.getNode(), sourceNode, sinkNode,
45+
"XML parsing depends on a $@ without guarding against external entity expansion.",
46+
sourceNode.getNode(), "user-provided value"
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
use libxml::bindings::{xmlReadMemory, XML_PARSE_NOENT};
2+
use std::ffi::CString;
3+
4+
fn parse_user_xml(user_input: &str) {
5+
let c_input = CString::new(user_input).unwrap();
6+
// BAD: external entity expansion is enabled via XML_PARSE_NOENT
7+
unsafe {
8+
xmlReadMemory(
9+
c_input.as_ptr(),
10+
c_input.as_bytes().len() as i32,
11+
std::ptr::null(),
12+
std::ptr::null(),
13+
XML_PARSE_NOENT as i32,
14+
);
15+
}
16+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
use libxml::bindings::xmlReadMemory;
2+
use std::ffi::CString;
3+
4+
fn parse_user_xml(user_input: &str) {
5+
let c_input = CString::new(user_input).unwrap();
6+
// GOOD: safe options (0) disable external entity expansion
7+
unsafe {
8+
xmlReadMemory(
9+
c_input.as_ptr(),
10+
c_input.as_bytes().len() as i32,
11+
std::ptr::null(),
12+
std::ptr::null(),
13+
0,
14+
);
15+
}
16+
}

rust/ql/test/query-tests/security/CWE-611/Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
#select
2+
| main.rs:68:19:68:26 | user_xml | main.rs:132:20:132:33 | ...::args | main.rs:68:19:68:26 | user_xml | XML parsing depends on a $@ without guarding against external entity expansion. | main.rs:132:20:132:33 | ...::args | user-provided value |
3+
| main.rs:73:19:73:26 | user_xml | main.rs:132:20:132:33 | ...::args | main.rs:73:19:73:26 | user_xml | XML parsing depends on a $@ without guarding against external entity expansion. | main.rs:132:20:132:33 | ...::args | user-provided value |
4+
| main.rs:78:19:78:26 | user_xml | main.rs:132:20:132:33 | ...::args | main.rs:78:19:78:26 | user_xml | XML parsing depends on a $@ without guarding against external entity expansion. | main.rs:132:20:132:33 | ...::args | user-provided value |
5+
| main.rs:83:17:83:29 | user_filename | main.rs:133:25:133:38 | ...::args | main.rs:83:17:83:29 | user_filename | XML parsing depends on a $@ without guarding against external entity expansion. | main.rs:133:25:133:38 | ...::args | user-provided value |
6+
| main.rs:88:16:88:23 | user_xml | main.rs:132:20:132:33 | ...::args | main.rs:88:16:88:23 | user_xml | XML parsing depends on a $@ without guarding against external entity expansion. | main.rs:132:20:132:33 | ...::args | user-provided value |
7+
| main.rs:93:42:93:49 | user_xml | main.rs:132:20:132:33 | ...::args | main.rs:93:42:93:49 | user_xml | XML parsing depends on a $@ without guarding against external entity expansion. | main.rs:132:20:132:33 | ...::args | user-provided value |
8+
| main.rs:100:9:100:16 | user_xml | main.rs:132:20:132:33 | ...::args | main.rs:100:9:100:16 | user_xml | XML parsing depends on a $@ without guarding against external entity expansion. | main.rs:132:20:132:33 | ...::args | user-provided value |
9+
| main.rs:110:19:110:26 | user_xml | main.rs:132:20:132:33 | ...::args | main.rs:110:19:110:26 | user_xml | XML parsing depends on a $@ without guarding against external entity expansion. | main.rs:132:20:132:33 | ...::args | user-provided value |
10+
edges
11+
| main.rs:66:25:66:38 | ...: ... [&ref] | main.rs:68:19:68:26 | user_xml | provenance | |
12+
| main.rs:71:27:71:40 | ...: ... [&ref] | main.rs:73:19:73:26 | user_xml | provenance | |
13+
| main.rs:76:28:76:41 | ...: ... [&ref] | main.rs:78:19:78:26 | user_xml | provenance | |
14+
| main.rs:81:27:81:45 | ...: ... [&ref] | main.rs:83:17:83:29 | user_filename | provenance | |
15+
| main.rs:86:26:86:39 | ...: ... [&ref] | main.rs:88:16:88:23 | user_xml | provenance | |
16+
| main.rs:91:31:91:44 | ...: ... [&ref] | main.rs:93:42:93:49 | user_xml | provenance | |
17+
| main.rs:96:34:96:47 | ...: ... [&ref] | main.rs:100:9:100:16 | user_xml | provenance | |
18+
| main.rs:108:29:108:42 | ...: ... [&ref] | main.rs:110:19:110:26 | user_xml | provenance | |
19+
| main.rs:132:9:132:16 | user_xml | main.rs:135:27:135:34 | user_xml | provenance | |
20+
| main.rs:132:9:132:16 | user_xml | main.rs:136:29:136:36 | user_xml | provenance | |
21+
| main.rs:132:9:132:16 | user_xml | main.rs:137:30:137:37 | user_xml | provenance | |
22+
| main.rs:132:9:132:16 | user_xml | main.rs:139:28:139:35 | user_xml | provenance | |
23+
| main.rs:132:9:132:16 | user_xml | main.rs:140:33:140:40 | user_xml | provenance | |
24+
| main.rs:132:9:132:16 | user_xml | main.rs:141:36:141:43 | user_xml | provenance | |
25+
| main.rs:132:9:132:16 | user_xml | main.rs:142:31:142:38 | user_xml | provenance | |
26+
| main.rs:132:20:132:33 | ...::args | main.rs:132:20:132:35 | ...::args(...) [element] | provenance | Src:MaD:488 |
27+
| main.rs:132:20:132:35 | ...::args(...) [element] | main.rs:132:20:132:42 | ... .nth(...) [Some] | provenance | MaD:440 |
28+
| main.rs:132:20:132:42 | ... .nth(...) [Some] | main.rs:132:20:132:62 | ... .unwrap_or_default() | provenance | MaD:9229 |
29+
| main.rs:132:20:132:62 | ... .unwrap_or_default() | main.rs:132:9:132:16 | user_xml | provenance | |
30+
| main.rs:133:9:133:21 | user_filename | main.rs:138:29:138:41 | user_filename | provenance | |
31+
| main.rs:133:25:133:38 | ...::args | main.rs:133:25:133:40 | ...::args(...) [element] | provenance | Src:MaD:488 |
32+
| main.rs:133:25:133:40 | ...::args(...) [element] | main.rs:133:25:133:47 | ... .nth(...) [Some] | provenance | MaD:440 |
33+
| main.rs:133:25:133:47 | ... .nth(...) [Some] | main.rs:133:25:133:67 | ... .unwrap_or_default() | provenance | MaD:9229 |
34+
| main.rs:133:25:133:67 | ... .unwrap_or_default() | main.rs:133:9:133:21 | user_filename | provenance | |
35+
| main.rs:135:26:135:34 | &user_xml [&ref] | main.rs:66:25:66:38 | ...: ... [&ref] | provenance | |
36+
| main.rs:135:27:135:34 | user_xml | main.rs:135:26:135:34 | &user_xml [&ref] | provenance | |
37+
| main.rs:136:28:136:36 | &user_xml [&ref] | main.rs:71:27:71:40 | ...: ... [&ref] | provenance | |
38+
| main.rs:136:29:136:36 | user_xml | main.rs:136:28:136:36 | &user_xml [&ref] | provenance | |
39+
| main.rs:137:29:137:37 | &user_xml [&ref] | main.rs:76:28:76:41 | ...: ... [&ref] | provenance | |
40+
| main.rs:137:30:137:37 | user_xml | main.rs:137:29:137:37 | &user_xml [&ref] | provenance | |
41+
| main.rs:138:28:138:41 | &user_filename [&ref] | main.rs:81:27:81:45 | ...: ... [&ref] | provenance | |
42+
| main.rs:138:29:138:41 | user_filename | main.rs:138:28:138:41 | &user_filename [&ref] | provenance | |
43+
| main.rs:139:27:139:35 | &user_xml [&ref] | main.rs:86:26:86:39 | ...: ... [&ref] | provenance | |
44+
| main.rs:139:28:139:35 | user_xml | main.rs:139:27:139:35 | &user_xml [&ref] | provenance | |
45+
| main.rs:140:32:140:40 | &user_xml [&ref] | main.rs:91:31:91:44 | ...: ... [&ref] | provenance | |
46+
| main.rs:140:33:140:40 | user_xml | main.rs:140:32:140:40 | &user_xml [&ref] | provenance | |
47+
| main.rs:141:35:141:43 | &user_xml [&ref] | main.rs:96:34:96:47 | ...: ... [&ref] | provenance | |
48+
| main.rs:141:36:141:43 | user_xml | main.rs:141:35:141:43 | &user_xml [&ref] | provenance | |
49+
| main.rs:142:30:142:38 | &user_xml [&ref] | main.rs:108:29:108:42 | ...: ... [&ref] | provenance | |
50+
| main.rs:142:31:142:38 | user_xml | main.rs:142:30:142:38 | &user_xml [&ref] | provenance | |
51+
nodes
52+
| main.rs:66:25:66:38 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
53+
| main.rs:68:19:68:26 | user_xml | semmle.label | user_xml |
54+
| main.rs:71:27:71:40 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
55+
| main.rs:73:19:73:26 | user_xml | semmle.label | user_xml |
56+
| main.rs:76:28:76:41 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
57+
| main.rs:78:19:78:26 | user_xml | semmle.label | user_xml |
58+
| main.rs:81:27:81:45 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
59+
| main.rs:83:17:83:29 | user_filename | semmle.label | user_filename |
60+
| main.rs:86:26:86:39 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
61+
| main.rs:88:16:88:23 | user_xml | semmle.label | user_xml |
62+
| main.rs:91:31:91:44 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
63+
| main.rs:93:42:93:49 | user_xml | semmle.label | user_xml |
64+
| main.rs:96:34:96:47 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
65+
| main.rs:100:9:100:16 | user_xml | semmle.label | user_xml |
66+
| main.rs:108:29:108:42 | ...: ... [&ref] | semmle.label | ...: ... [&ref] |
67+
| main.rs:110:19:110:26 | user_xml | semmle.label | user_xml |
68+
| main.rs:132:9:132:16 | user_xml | semmle.label | user_xml |
69+
| main.rs:132:20:132:33 | ...::args | semmle.label | ...::args |
70+
| main.rs:132:20:132:35 | ...::args(...) [element] | semmle.label | ...::args(...) [element] |
71+
| main.rs:132:20:132:42 | ... .nth(...) [Some] | semmle.label | ... .nth(...) [Some] |
72+
| main.rs:132:20:132:62 | ... .unwrap_or_default() | semmle.label | ... .unwrap_or_default() |
73+
| main.rs:133:9:133:21 | user_filename | semmle.label | user_filename |
74+
| main.rs:133:25:133:38 | ...::args | semmle.label | ...::args |
75+
| main.rs:133:25:133:40 | ...::args(...) [element] | semmle.label | ...::args(...) [element] |
76+
| main.rs:133:25:133:47 | ... .nth(...) [Some] | semmle.label | ... .nth(...) [Some] |
77+
| main.rs:133:25:133:67 | ... .unwrap_or_default() | semmle.label | ... .unwrap_or_default() |
78+
| main.rs:135:26:135:34 | &user_xml [&ref] | semmle.label | &user_xml [&ref] |
79+
| main.rs:135:27:135:34 | user_xml | semmle.label | user_xml |
80+
| main.rs:136:28:136:36 | &user_xml [&ref] | semmle.label | &user_xml [&ref] |
81+
| main.rs:136:29:136:36 | user_xml | semmle.label | user_xml |
82+
| main.rs:137:29:137:37 | &user_xml [&ref] | semmle.label | &user_xml [&ref] |
83+
| main.rs:137:30:137:37 | user_xml | semmle.label | user_xml |
84+
| main.rs:138:28:138:41 | &user_filename [&ref] | semmle.label | &user_filename [&ref] |
85+
| main.rs:138:29:138:41 | user_filename | semmle.label | user_filename |
86+
| main.rs:139:27:139:35 | &user_xml [&ref] | semmle.label | &user_xml [&ref] |
87+
| main.rs:139:28:139:35 | user_xml | semmle.label | user_xml |
88+
| main.rs:140:32:140:40 | &user_xml [&ref] | semmle.label | &user_xml [&ref] |
89+
| main.rs:140:33:140:40 | user_xml | semmle.label | user_xml |
90+
| main.rs:141:35:141:43 | &user_xml [&ref] | semmle.label | &user_xml [&ref] |
91+
| main.rs:141:36:141:43 | user_xml | semmle.label | user_xml |
92+
| main.rs:142:30:142:38 | &user_xml [&ref] | semmle.label | &user_xml [&ref] |
93+
| main.rs:142:31:142:38 | user_xml | semmle.label | user_xml |
94+
subpaths
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
query: queries/security/CWE-611/Xxe.ql
2+
postprocess:
3+
- utils/test/InlineExpectationsTestQuery.ql

0 commit comments

Comments
 (0)