Skip to content

Commit 86eb949

Browse files
authored
Merge pull request #20902 from paldepind/rust/xss-query
Rust: Add new query for XSS vulnerabilities
2 parents 0c358ac + 97dad2d commit 86eb949

File tree

28 files changed

+5212
-1
lines changed

28 files changed

+5212
-1
lines changed

rust/ql/integration-tests/query-suite/rust-code-scanning.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ ql/rust/ql/src/queries/diagnostics/UnextractedElements.ql
1010
ql/rust/ql/src/queries/diagnostics/UnresolvedMacroCalls.ql
1111
ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
1212
ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
13+
ql/rust/ql/src/queries/security/CWE-079/XSS.ql
1314
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
1415
ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql
1516
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql

rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ ql/rust/ql/src/queries/diagnostics/UnextractedElements.ql
1010
ql/rust/ql/src/queries/diagnostics/UnresolvedMacroCalls.ql
1111
ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
1212
ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
13+
ql/rust/ql/src/queries/security/CWE-079/XSS.ql
1314
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
1415
ql/rust/ql/src/queries/security/CWE-117/LogInjection.ql
1516
ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql

rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ ql/rust/ql/src/queries/diagnostics/UnextractedElements.ql
1010
ql/rust/ql/src/queries/diagnostics/UnresolvedMacroCalls.ql
1111
ql/rust/ql/src/queries/security/CWE-020/RegexInjection.ql
1212
ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
13+
ql/rust/ql/src/queries/security/CWE-079/XSS.ql
1314
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
1415
ql/rust/ql/src/queries/security/CWE-117/LogInjection.ql
1516
ql/rust/ql/src/queries/security/CWE-295/DisabledCertificateCheck.ql

rust/ql/lib/codeql/rust/frameworks/actix-web.model.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ extensions:
66
- ["<actix_web::route::Route>::to", "Argument[0].Parameter[0..7]", "remote", "manual"]
77
# Actix attributes such as `get` expand to this `to` call on the handler.
88
- ["<actix_web::resource::Resource>::to", "Argument[0].Parameter[0..7]", "remote", "manual"]
9+
- addsTo:
10+
pack: codeql/rust-all
11+
extensible: sinkModel
12+
data:
13+
- ["<actix_web::types::html::Html>::new", "Argument[0]", "html-injection", "manual"]
914
- addsTo:
1015
pack: codeql/rust-all
1116
extensible: summaryModel

rust/ql/lib/codeql/rust/frameworks/warp.model.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,9 @@ extensions:
55
data:
66
- ["<_ as warp::filter::Filter>::then", "Argument[0].Parameter[0..7]", "remote", "manual"]
77
- ["<_ as warp::filter::Filter>::map", "Argument[0].Parameter[0..7]", "remote", "manual"]
8-
- ["<_ as warp::filter::Filter>::and_then", "Argument[0].Parameter[0..7]", "remote", "manual"]
8+
- ["<_ as warp::filter::Filter>::and_then", "Argument[0].Parameter[0..7]", "remote", "manual"]
9+
- addsTo:
10+
pack: codeql/rust-all
11+
extensible: sinkModel
12+
data:
13+
- ["warp::reply::html", "Argument[0]", "html-injection", "manual"]
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Provides classes and predicates for reasoning about cross-site scripting (XSS)
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+
private import codeql.util.Unit
11+
private import codeql.rust.security.Barriers as Barriers
12+
13+
/**
14+
* Provides default sources, sinks and barriers for detecting XSS
15+
* vulnerabilities, as well as extension points for adding your own.
16+
*/
17+
module Xss {
18+
/**
19+
* A data flow source for XSS vulnerabilities.
20+
*/
21+
abstract class Source extends DataFlow::Node { }
22+
23+
/**
24+
* A data flow sink for XSS vulnerabilities.
25+
*/
26+
abstract class Sink extends QuerySink::Range {
27+
override string getSinkType() { result = "Xss" }
28+
}
29+
30+
/**
31+
* A barrier for XSS vulnerabilities.
32+
*/
33+
abstract class Barrier extends DataFlow::Node { }
34+
35+
/**
36+
* An active threat-model source, considered as a flow source.
37+
*/
38+
private class ActiveThreatModelSourceAsSource extends Source, ActiveThreatModelSource { }
39+
40+
/**
41+
* A sink for XSS from model data.
42+
*/
43+
private class ModelsAsDataSink extends Sink {
44+
ModelsAsDataSink() { sinkNode(this, "html-injection") }
45+
}
46+
47+
/**
48+
* A barrier for XSS vulnerabilities for nodes whose type is a
49+
* numeric or boolean type, which is unlikely to expose any vulnerability.
50+
*/
51+
private class NumericTypeBarrier extends Barrier instanceof Barriers::NumericTypeBarrier { }
52+
53+
/** A call to a function with "escape" or "encode" in its name. */
54+
private class HeuristicHtmlEncodingBarrier extends Barrier {
55+
HeuristicHtmlEncodingBarrier() {
56+
exists(Call fc |
57+
fc.getStaticTarget().getName().getText().regexpMatch(".*(escape|encode).*") and
58+
fc.getArgument(_) = this.asExpr()
59+
)
60+
}
61+
}
62+
}
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/xss`, to detect cross-site scripting security vulnerabilities.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>Directly writing user input (for example, an HTTP request parameter) to a webpage,
8+
without properly sanitizing the input first, allows for a cross-site
9+
scripting vulnerability.</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>To guard against cross-site scripting, consider encoding/escaping the untrusted
14+
input before including it in the HTML.</p>
15+
</recommendation>
16+
17+
<example>
18+
19+
<p>The following example shows a simple web handler that writes a URL path parameter
20+
directly to an HTML response, leaving the website vulnerable to cross-site
21+
scripting:</p>
22+
23+
<sample src="XSSBad.rs" />
24+
25+
<p>To fix this vulnerability, the user input should be HTML-encoded before being
26+
included in the response. In the following example, <code>encode_text</code> from
27+
the <a href="https://docs.rs/html-escape/latest/html_escape/index.html">html_escape</a>
28+
crate is used to achieve this:</p>
29+
30+
<sample src="XSSGood.rs" />
31+
32+
</example>
33+
34+
<references>
35+
<li>
36+
OWASP:
37+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html">
38+
Cross Site Scripting Prevention Cheat Sheet</a>.
39+
</li>
40+
<li>
41+
Wikipedia: <a href="https://en.wikipedia.org/wiki/Cross-site_scripting">Cross-site scripting</a>.
42+
</li>
43+
<li>
44+
OWASP:
45+
<a href="https://owasp.org/www-community/attacks/xss/">Cross Site Scripting (XSS)</a>.
46+
</li>
47+
</references>
48+
</qhelp>
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
/**
2+
* @name Cross-site scripting
3+
* @description Writing user input directly to a webpage
4+
* allows for a cross-site scripting vulnerability.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 6.1
8+
* @precision high
9+
* @id rust/xss
10+
* @tags security
11+
* external/cwe/cwe-079
12+
* external/cwe/cwe-116
13+
*/
14+
15+
import rust
16+
import codeql.rust.dataflow.DataFlow
17+
import codeql.rust.dataflow.TaintTracking
18+
import codeql.rust.security.XssExtensions
19+
20+
/**
21+
* A taint configuration for tainted data that reaches an XSS sink.
22+
*/
23+
module XssConfig implements DataFlow::ConfigSig {
24+
import Xss
25+
26+
predicate isSource(DataFlow::Node node) { node instanceof Source }
27+
28+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
29+
30+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
31+
32+
predicate observeDiffInformedIncrementalMode() { any() }
33+
}
34+
35+
module XssFlow = TaintTracking::Global<XssConfig>;
36+
37+
import XssFlow::PathGraph
38+
39+
from XssFlow::PathNode sourceNode, XssFlow::PathNode sinkNode
40+
where XssFlow::flowPath(sourceNode, sinkNode)
41+
select sinkNode.getNode(), sourceNode, sinkNode, "Cross-site scripting vulnerability due to a $@.",
42+
sourceNode.getNode(), "user-provided value"
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
use actix_web::{web, HttpResponse, Result};
2+
3+
// BAD: User input is directly included in HTML response without sanitization
4+
async fn vulnerable_handler(path: web::Path<String>) -> impl Responder {
5+
let user_input = path.into_inner();
6+
7+
let html = format!(
8+
r#"
9+
<!DOCTYPE html>
10+
<html>
11+
<head><title>Welcome</title></head>
12+
<body>
13+
<h1>Hello, {}!</h1>
14+
</body>
15+
</html>
16+
"#,
17+
user_input
18+
);
19+
20+
Html::new(html) // Unsafe: User input included directly in the response
21+
}

0 commit comments

Comments
 (0)