Skip to content

Commit b4c0b1b

Browse files
RasmusWLRasmusWL
committed
Python: Port py/reflective-xss to use proper source/sink customization
1 parent 62e4445 commit b4c0b1b

File tree

3 files changed

+106
-29
lines changed

3 files changed

+106
-29
lines changed

‎python/ql/src/Security/CWE-079/ReflectedXss.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import python
1717
import semmle.python.security.dataflow.ReflectedXSS
1818
import DataFlow::PathGraph
1919

20-
from ReflectedXssConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
20+
from ReflectedXSS::Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
2121
where config.hasFlowPath(source, sink)
2222
select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@.",
2323
source.getNode(), "a user-provided value"

‎python/ql/src/semmle/python/security/dataflow/ReflectedXSS.qll

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,42 @@
11
/**
2-
* Provides a taint-tracking configuration for detecting reflected server-side
3-
* cross-site scripting vulnerabilities.
2+
* Provides a taint-tracking configuration for detecting "reflected server-side cross-site scripting" vulnerabilities.
3+
*
4+
* Note, for performance reasons: only import this file if
5+
* `ReflectedXSS::Configuration` is needed, otherwise
6+
* `ReflectedXSSCustomizations` should be imported instead.
47
*/
58

6-
import python
9+
private import python
710
import semmle.python.dataflow.new.DataFlow
811
import semmle.python.dataflow.new.TaintTracking
9-
import semmle.python.Concepts
10-
import semmle.python.dataflow.new.RemoteFlowSources
11-
import semmle.python.dataflow.new.BarrierGuards
1212

1313
/**
14-
* A taint-tracking configuration for detecting reflected server-side cross-site
15-
* scripting vulnerabilities.
14+
* Provides a taint-tracking configuration for detecting "reflected server-side cross-site scripting" vulnerabilities.
1615
*/
17-
class ReflectedXssConfiguration extends TaintTracking::Configuration {
18-
ReflectedXssConfiguration() { this = "ReflectedXssConfiguration" }
16+
module ReflectedXSS {
17+
import ReflectedXSSCustomizations::ReflectedXSS
1918

20-
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
19+
/**
20+
* A taint-tracking configuration for detecting "reflected server-side cross-site scripting" vulnerabilities.
21+
*/
22+
class Configuration extends TaintTracking::Configuration {
23+
Configuration() { this = "ReflectedXSS" }
2124

22-
override predicate isSink(DataFlow::Node sink) {
23-
exists(HTTP::Server::HttpResponse response |
24-
response.getMimetype().toLowerCase() = "text/html" and
25-
sink = response.getBody()
26-
)
27-
}
25+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
2826

29-
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
30-
guard instanceof StringConstCompare
31-
}
27+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
3228

33-
// TODO: For now, since there is not an `isSanitizingStep` member-predicate part of a
34-
// `TaintTracking::Configuration`, we use treat the output is a taint-sanitizer. This
35-
// is slightly imprecise, which you can see in the `m_unsafe + SAFE` test-case in
36-
// python/ql/test/library-tests/frameworks/markupsafe/taint_test.py
37-
//
38-
// However, it is better than `getAnInput()`. Due to use-use flow, that would remove
39-
// the taint-flow to `SINK()` in `some_escape(tainted); SINK(tainted)`.
40-
override predicate isSanitizer(DataFlow::Node node) { node = any(HtmlEscaping esc).getOutput() }
29+
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
30+
31+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
32+
guard instanceof SanitizerGuard
33+
}
34+
}
4135
}
36+
37+
/**
38+
* DEPRECATED: Don't extend this class for customization, since this will lead to bad
39+
* performance, instead use the new `ReflectedXSSCustomizations.qll` file, and extend
40+
* its' classes.
41+
*/
42+
deprecated class ReflectedXssConfiguration = ReflectedXSS::Configuration;

‎python/ql/src/semmle/python/security/dataflow/ReflectedXSSCustomizations.qll

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for detecting
3+
* "reflected server-side cross-site scripting"
4+
* vulnerabilities, as well as extension points for adding your own.
5+
*/
6+
7+
private import python
8+
private import semmle.python.dataflow.new.DataFlow
9+
private import semmle.python.Concepts
10+
private import semmle.python.dataflow.new.RemoteFlowSources
11+
private import semmle.python.dataflow.new.BarrierGuards
12+
13+
/**
14+
* Provides default sources, sinks and sanitizers for detecting
15+
* "reflected server-side cross-site scripting"
16+
* vulnerabilities, as well as extension points for adding your own.
17+
*/
18+
module ReflectedXSS {
19+
/**
20+
* A data flow source for "reflected server-side cross-site scripting" vulnerabilities.
21+
*/
22+
abstract class Source extends DataFlow::Node { }
23+
24+
/**
25+
* A data flow sink for "reflected server-side cross-site scripting" vulnerabilities.
26+
*/
27+
abstract class Sink extends DataFlow::Node { }
28+
29+
/**
30+
* A sanitizer for "reflected server-side cross-site scripting" vulnerabilities.
31+
*/
32+
abstract class Sanitizer extends DataFlow::Node { }
33+
34+
/**
35+
* A sanitizer guard for "reflected server-side cross-site scripting" vulnerabilities.
36+
*/
37+
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
38+
39+
/**
40+
* A source of remote user input, considered as a flow source.
41+
*/
42+
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
43+
44+
/**
45+
* The body of a HTTP response that will be returned from a server, considered as a flow sink.
46+
*/
47+
class ServerHttpResponseBodyAsSink extends Sink {
48+
ServerHttpResponseBodyAsSink() {
49+
exists(HTTP::Server::HttpResponse response |
50+
response.getMimetype().toLowerCase() = "text/html" and
51+
this = response.getBody()
52+
)
53+
}
54+
}
55+
56+
/**
57+
* An HTML escaping, considered as a sanitizer.
58+
*/
59+
class HtmlEscapingAsSanitizer extends Sanitizer {
60+
HtmlEscapingAsSanitizer() {
61+
// TODO: For now, since there is not an `isSanitizingStep` member-predicate part of a
62+
// `TaintTracking::Configuration`, we use treat the output is a taint-sanitizer. This
63+
// is slightly imprecise, which you can see in the `m_unsafe + SAFE` test-case in
64+
// python/ql/test/library-tests/frameworks/markupsafe/taint_test.py
65+
//
66+
// However, it is better than `getAnInput()`. Due to use-use flow, that would remove
67+
// the taint-flow to `SINK()` in `some_escape(tainted); SINK(tainted)`.
68+
this = any(HtmlEscaping esc).getOutput()
69+
}
70+
}
71+
72+
/**
73+
* A comparison with a constant string, considered as a sanitizer-guard.
74+
*/
75+
class StringConstCompareAsSanitizerGuard extends SanitizerGuard, StringConstCompare { }
76+
}

0 commit comments

Comments
 (0)