Skip to content

Commit abe7aee

Browse files
esbenaesbena
authored
Merge pull request #2643 from esbena/js/unsafe-jquery
JS: add query js/unsafe-jquery-plugin
2 parents ecad925 + 1ec8fa2 commit abe7aee

12 files changed

Lines changed: 903 additions & 1 deletion

File tree

‎change-notes/1.24/analysis-javascript.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
| Regular expression always matches (`js/regex/always-matches`) | correctness, regular-expressions | Highlights regular expression checks that trivially succeed by matching an empty substring. Results are shown on LGTM by default. |
3939
| Missing await (`js/missing-await`) | correctness | Highlights expressions that operate directly on a promise object in a nonsensical way, instead of awaiting its result. Results are shown on LGTM by default. |
4040
| Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive copying operations that are susceptible to prototype pollution. Results are shown on LGTM by default. |
41+
| Unsafe jQuery plugin (`js/unsafe-jquery-plugin`) | Highlights potential XSS vulnerabilities in unsafely designed jQuery plugins. Results are shown on LGTM by default. |
4142

4243
## Changes to existing queries
4344

‎javascript/config/suites/javascript/security‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
+ semmlecode-javascript-queries/Security/CWE-079/ReflectedXss.ql: /Security/CWE/CWE-079
1515
+ semmlecode-javascript-queries/Security/CWE-079/StoredXss.ql: /Security/CWE/CWE-079
1616
+ semmlecode-javascript-queries/Security/CWE-079/Xss.ql: /Security/CWE/CWE-079
17+
+ semmlecode-javascript-queries/Security/CWE-079/UnsafeJQueryPlugin.ql: /Security/CWE/CWE-079
1718
+ semmlecode-javascript-queries/Security/CWE-089/SqlInjection.ql: /Security/CWE/CWE-089
1819
+ semmlecode-javascript-queries/Security/CWE-094/CodeInjection.ql: /Security/CWE/CWE-094
1920
+ semmlecode-javascript-queries/Security/CWE-094/UnsafeDynamicMethodAccess.ql: /Security/CWE/CWE-094

‎javascript/ql/src/Security/CWE-079/UnsafeJQueryPlugin.qhelp‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
9+
Library plugins, such as those for the jQuery library, are often
10+
configurable through options provided by the clients of the
11+
plugin.
12+
13+
14+
Clients, however, do not know the implementation details
15+
of the plugin, so it is important to document the capabilities of each
16+
option. The documentation for the plugin options that the client is
17+
responsible for sanitizing is of particular importance.
18+
19+
Otherwise, the plugin may write user input (for example, a URL query
20+
parameter) to a web page without properly sanitizing it first,
21+
which allows for a cross-site scripting vulnerability in the client
22+
application through dynamic HTML construction.
23+
24+
</p>
25+
</overview>
26+
27+
<recommendation>
28+
<p>
29+
30+
Document all options that can lead to cross-site scripting
31+
attacks, and guard against unsafe inputs where dynamic HTML
32+
construction is not intended.
33+
34+
</p>
35+
</recommendation>
36+
37+
<example>
38+
<p>
39+
40+
The following example shows a jQuery plugin that selects a
41+
DOM element, and copies its text content to another DOM element. The
42+
selection is performed by using the plugin option
43+
<code>sourceSelector</code> as a CSS selector.
44+
45+
</p>
46+
47+
<sample src="examples/UnsafeJQueryPlugin.js" />
48+
49+
<p>
50+
51+
This is, however, not a safe plugin, since the call to
52+
<code>jQuery</code> interprets <code>sourceSelector</code> as HTML if
53+
it is a string that starts with <code>&lt;</code>.
54+
55+
</p>
56+
57+
<p>
58+
59+
Instead of documenting that the client is responsible for
60+
sanitizing <code>sourceSelector</code>, the plugin can use
61+
<code>jQuery.find</code> to always interpret
62+
<code>sourceSelector</code> as a CSS selector:
63+
64+
</p>
65+
66+
<sample src="examples/UnsafeJQueryPlugin_safe.js" />
67+
68+
69+
</example>
70+
71+
<references>
72+
<li>
73+
OWASP:
74+
<a href="https://www.owasp.org/index.php/DOM_based_XSS_Prevention_Cheat_Sheet">DOM based
75+
XSS Prevention Cheat Sheet</a>.
76+
</li>
77+
<li>
78+
OWASP:
79+
<a href="https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet">XSS
80+
(Cross Site Scripting) Prevention Cheat Sheet</a>.
81+
</li>
82+
<li>
83+
OWASP
84+
<a href="https://www.owasp.org/index.php/DOM_Based_XSS">DOM Based XSS</a>.
85+
</li>
86+
<li>
87+
OWASP
88+
<a href="https://www.owasp.org/index.php/Types_of_Cross-Site_Scripting">Types of Cross-Site
89+
Scripting</a>.
90+
</li>
91+
<li>
92+
Wikipedia: <a href="http://en.wikipedia.org/wiki/Cross-site_scripting">Cross-site scripting</a>.
93+
</li>
94+
<li>
95+
jQuery: <a href="https://learn.jquery.com/plugins/basic-plugin-creation/">Plugin creation</a>.
96+
</li>
97+
<li>
98+
Bootstrap: <a href="https://github.com/twbs/bootstrap/pull/27047">XSS vulnerable bootstrap plugins</a>.
99+
</li>
100+
</references>
101+
</qhelp>

‎javascript/ql/src/Security/CWE-079/UnsafeJQueryPlugin.ql‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
/**
2+
* @name Unsafe jQuery plugin
3+
* @description A jQuery plugin that unintentionally constructs HTML from some of its options may be unsafe to use for clients.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @precision high
7+
* @id js/unsafe-jquery-plugin
8+
* @tags security
9+
* external/cwe/cwe-079
10+
* external/cwe/cwe-116
11+
* frameworks/jquery
12+
*/
13+
14+
import javascript
15+
import semmle.javascript.security.dataflow.UnsafeJQueryPlugin::UnsafeJQueryPlugin
16+
import DataFlow::PathGraph
17+
18+
from
19+
Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, JQueryPluginMethod plugin
20+
where
21+
cfg.hasFlowPath(source, sink) and
22+
source.getNode().(Source).getPlugin() = plugin and
23+
not isLikelyIntentionalHtmlSink(plugin, sink.getNode())
24+
select sink.getNode(), source, sink, "Potential XSS vulnerability in the $@.", plugin,
25+
"'$.fn." + plugin.getPluginName() + "' plugin"

‎javascript/ql/src/Security/CWE-079/examples/UnsafeJQueryPlugin.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
jQuery.fn.copyText = function(options) {
2+
// BAD may evaluate `options.sourceSelector` as HTML
3+
var source = jQuery(options.sourceSelector),
4+
text = source.text();
5+
jQuery(this).text(text);
6+
}

‎javascript/ql/src/Security/CWE-079/examples/UnsafeJQueryPlugin_safe.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
jQuery.fn.copyText = function(options) {
2+
// GOOD may not evaluate `options.sourceSelector` as HTML
3+
var source = jQuery.find(options.sourceSelector),
4+
text = source.text();
5+
jQuery(this).text(text);
6+
}

‎javascript/ql/src/semmle/javascript/security/dataflow/UnsafeJQueryPlugin.qll‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about DOM-based
3+
* cross-site scripting vulnerabilities in unsafe jQuery plugins.
4+
*/
5+
6+
import javascript
7+
import semmle.javascript.security.dataflow.Xss
8+
9+
module UnsafeJQueryPlugin {
10+
import UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin
11+
12+
/**
13+
* A taint-tracking configuration for reasoning about XSS in unsafe jQuery plugins.
14+
*/
15+
class Configuration extends TaintTracking::Configuration {
16+
Configuration() { this = "UnsafeJQueryPlugin" }
17+
18+
override predicate isSource(DataFlow::Node source) { source instanceof Source }
19+
20+
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
21+
22+
override predicate isSanitizer(DataFlow::Node node) {
23+
super.isSanitizer(node)
24+
or
25+
node instanceof DomBasedXss::Sanitizer
26+
or
27+
node instanceof Sanitizer
28+
}
29+
30+
override predicate isAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
31+
// jQuery plugins tend to be implemented as classes that store data in fields initialized by the constructor.
32+
DataFlow::localFieldStep(src, sink)
33+
}
34+
35+
override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
36+
// prefixing prevents forced html/css confusion:
37+
38+
// prefixing through concatenation:
39+
StringConcatenation::taintStep(pred, succ, _, any(int i | i >= 1))
40+
or
41+
// prefixing through a poor-mans templating system:
42+
exists(DataFlow::MethodCallNode replace |
43+
replace = succ and
44+
pred = replace.getArgument(1) and
45+
replace.getMethodName() = "replace"
46+
)
47+
}
48+
49+
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode node) {
50+
super.isSanitizerGuard(node) or
51+
node instanceof IsElementSanitizer or
52+
node instanceof PropertyPresenceSanitizer
53+
}
54+
}
55+
}

0 commit comments

Comments
 (0)