Skip to content

Commit a2084f8

Browse files
alexrfordalexrford
committed
rb/stored-xss structure and initial implementation (FileSystemReadAccess sources)
1 parent 1c08592 commit a2084f8

7 files changed

Lines changed: 165 additions & 16 deletions

File tree

‎ql/lib/codeql/ruby/security/ReflectedXSSQuery.qll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* Note, for performance reasons: only import this file if
55
* `ReflectedXSS::Configuration` is needed, otherwise
6-
* `ReflectedXSSCustomizations` should be imported instead.
6+
* `XSS::ReflectedXSS` should be imported instead.
77
*/
88

99
private import ruby
@@ -14,7 +14,7 @@ import codeql.ruby.TaintTracking
1414
* Provides a taint-tracking configuration for detecting "reflected server-side cross-site scripting" vulnerabilities.
1515
*/
1616
module ReflectedXSS {
17-
import ReflectedXSSCustomizations::ReflectedXSS
17+
import XSS::ReflectedXSS
1818

1919
/**
2020
* A taint-tracking configuration for detecting "reflected server-side cross-site scripting" vulnerabilities.

‎ql/lib/codeql/ruby/security/StoredXSSQuery.qll‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about stored
3+
* cross-site scripting vulnerabilities.
4+
*
5+
* Note, for performance reasons: only import this file if
6+
* `StoredXSS::Configuration` is needed, otherwise
7+
* `XSS::StoredXSS` should be imported instead.
8+
*/
9+
10+
import ruby
11+
import codeql.ruby.DataFlow
12+
import codeql.ruby.TaintTracking
13+
14+
module StoredXSS {
15+
import XSS::StoredXSS
16+
17+
/**
18+
* A taint-tracking configuration for reasoning about Stored XSS.
19+
*/
20+
class Configuration extends TaintTracking::Configuration {
21+
Configuration() { this = "StoredXss" }
22+
23+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
24+
25+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
26+
27+
override predicate isSanitizer(DataFlow::Node node) {
28+
super.isSanitizer(node) or
29+
node instanceof Sanitizer
30+
}
31+
32+
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
33+
guard instanceof SanitizerGuard
34+
}
35+
36+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
37+
isAdditionalXSSTaintStep(node1, node2)
38+
}
39+
}
40+
}

ql/lib/codeql/ruby/security/ReflectedXSSCustomizations.qll renamed to ql/lib/codeql/ruby/security/XSS.qll

Lines changed: 92 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
/**
2+
* Provides classes and predicates used by the XSS queries.
3+
*/
4+
15
private import ruby
26
private import codeql.ruby.DataFlow
37
private import codeql.ruby.CFG
@@ -7,40 +11,34 @@ private import codeql.ruby.frameworks.ActionController
711
private import codeql.ruby.frameworks.ActionView
812
private import codeql.ruby.dataflow.RemoteFlowSources
913
private import codeql.ruby.dataflow.BarrierGuards
10-
import codeql.ruby.dataflow.internal.DataFlowDispatch
11-
private import codeql.ruby.typetracking.TypeTracker
14+
private import codeql.ruby.dataflow.internal.DataFlowDispatch
1215

1316
/**
1417
* 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.
18+
* "server-side cross-site scripting" vulnerabilities, as well as
19+
* extension points for adding your own.
1720
*/
18-
module ReflectedXSS {
21+
private module Shared {
1922
/**
20-
* A data flow source for "reflected server-side cross-site scripting" vulnerabilities.
23+
* A data flow source for "server-side cross-site scripting" vulnerabilities.
2124
*/
2225
abstract class Source extends DataFlow::Node { }
2326

2427
/**
25-
* A data flow sink for "reflected server-side cross-site scripting" vulnerabilities.
28+
* A data flow sink for "server-side cross-site scripting" vulnerabilities.
2629
*/
2730
abstract class Sink extends DataFlow::Node { }
2831

2932
/**
30-
* A sanitizer for "reflected server-side cross-site scripting" vulnerabilities.
33+
* A sanitizer for "server-side cross-site scripting" vulnerabilities.
3134
*/
3235
abstract class Sanitizer extends DataFlow::Node { }
3336

3437
/**
35-
* A sanitizer guard for "reflected server-side cross-site scripting" vulnerabilities.
38+
* A sanitizer guard for "server-side cross-site scripting" vulnerabilities.
3639
*/
3740
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
3841

39-
/**
40-
* A source of remote user input, considered as a flow source.
41-
*/
42-
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
43-
4442
private class ErbOutputMethodCallArgumentNode extends DataFlow::Node {
4543
private MethodCall call;
4644

@@ -198,3 +196,83 @@ module ReflectedXSS {
198196
)
199197
}
200198
}
199+
200+
/**
201+
* Provides default sources, sinks and sanitizers for detecting
202+
* "reflected cross-site scripting" vulnerabilities, as well as
203+
* extension points for adding your own.
204+
*/
205+
module ReflectedXSS {
206+
/** A data flow source for stored XSS vulnerabilities. */
207+
abstract class Source extends Shared::Source { }
208+
209+
/** A data flow sink for stored XSS vulnerabilities. */
210+
abstract class Sink extends Shared::Sink { }
211+
212+
/** A sanitizer for stored XSS vulnerabilities. */
213+
abstract class Sanitizer extends Shared::Sanitizer { }
214+
215+
/** A sanitizer guard for stored XSS vulnerabilities. */
216+
abstract class SanitizerGuard extends Shared::SanitizerGuard { }
217+
218+
// Consider all arbitrary XSS sinks to be reflected XSS sinks
219+
private class AnySink extends Sink instanceof Shared::Sink { }
220+
221+
// Consider all arbitrary XSS sanitizers to be reflected XSS sanitizers
222+
private class AnySanitizer extends Sanitizer instanceof Shared::Sanitizer { }
223+
224+
// Consider all arbitrary XSS sanitizer guards to be reflected XSS sanitizer guards
225+
private class AnySanitizerGuard extends SanitizerGuard instanceof Shared::SanitizerGuard {
226+
override predicate checks(CfgNode expr, boolean branch) {
227+
Shared::SanitizerGuard.super.checks(expr, branch)
228+
}
229+
}
230+
231+
// Consider all arbitrary XSS taint steps to be reflected XSS taint steps
232+
predicate isAdditionalXSSTaintStep = Shared::isAdditionalXSSTaintStep/2;
233+
234+
/**
235+
* A source of remote user input, considered as a flow source.
236+
*/
237+
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
238+
}
239+
240+
/**
241+
* Provides default sources, sinks and sanitizers for detecting
242+
* "stored server-side cross-site scripting" vulnerabilities, as well as
243+
* extension points for adding your own.
244+
*/
245+
module StoredXSS {
246+
/** A data flow source for stored XSS vulnerabilities. */
247+
abstract class Source extends Shared::Source { }
248+
249+
/** A data flow sink for stored XSS vulnerabilities. */
250+
abstract class Sink extends Shared::Sink { }
251+
252+
/** A sanitizer for stored XSS vulnerabilities. */
253+
abstract class Sanitizer extends Shared::Sanitizer { }
254+
255+
/** A sanitizer guard for stored XSS vulnerabilities. */
256+
abstract class SanitizerGuard extends Shared::SanitizerGuard { }
257+
258+
// Consider all arbitrary XSS sinks to be stored XSS sinks
259+
private class AnySink extends Sink instanceof Shared::Sink { }
260+
261+
// Consider all arbitrary XSS sanitizers to be stored XSS sanitizers
262+
private class AnySanitizer extends Sanitizer instanceof Shared::Sanitizer { }
263+
264+
// Consider all arbitrary XSS sanitizer guards to be stored XSS sanitizer guards
265+
private class AnySanitizerGuard extends SanitizerGuard instanceof Shared::SanitizerGuard {
266+
override predicate checks(CfgNode expr, boolean branch) {
267+
Shared::SanitizerGuard.super.checks(expr, branch)
268+
}
269+
}
270+
271+
// Consider all arbitrary XSS taint steps to be stored XSS taint steps
272+
predicate isAdditionalXSSTaintStep = Shared::isAdditionalXSSTaintStep/2;
273+
274+
/** A file read, considered as a flow source for stored XSS. */
275+
class FileSystemReadAccessAsSource extends Source instanceof FileSystemReadAccess { }
276+
// TODO: Consider `FileNameSource` flowing to script tag `src` attributes and similar
277+
// TODO: ORM database reads as sources
278+
}

‎ql/src/queries/security/cwe-079/ReflectedXSS.ql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* allows for a cross-site scripting vulnerability.
55
* @kind path-problem
66
* @problem.severity error
7+
* @security-severity 6.1
78
* @sub-severity high
89
* @precision high
910
* @id rb/reflected-xss

‎ql/src/queries/security/cwe-079/StoredXSS.ql‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @name Stored cross-site scripting
3+
* @description Using uncontrolled stored values in HTML allows for
4+
* a stored cross-site scripting vulnerability.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @security-severity 6.1
8+
* @precision high
9+
* @id rb/stored-xss
10+
* @tags security
11+
* external/cwe/cwe-079
12+
* external/cwe/cwe-116
13+
*/
14+
15+
import ruby
16+
import codeql.ruby.security.StoredXSSQuery
17+
import codeql.ruby.DataFlow
18+
import DataFlow::PathGraph
19+
20+
from StoredXSS::Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink
21+
where config.hasFlowPath(source, sink)
22+
select sink.getNode(), source, sink, "Cross-site scripting vulnerability due to $@",
23+
source.getNode(), "stored value"

‎ql/test/query-tests/security/cwe-079/StoredXSS.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
queries/security/cwe-079/StoredXSS.ql

‎ql/test/query-tests/security/cwe-079/app/controllers/foo/bars_controller.rb‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,10 @@ def show
2222
@html_escaped = ERB::Util.html_escape(params[:text])
2323
render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
2424
end
25+
26+
def show_stored
27+
dt = File.read("foo.txt")
28+
@instance_text = dt
29+
render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
30+
end
2531
end

0 commit comments

Comments
 (0)