Skip to content

Commit fef918a

Browse files
esbenaesbena
committed
JS: add query "Unsafe jQuery plugin"
1 parent d995d5a commit fef918a

12 files changed

Lines changed: 1069 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
@@ -28,6 +28,7 @@
2828
| 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. |
2929
| 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. |
3030
| 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. |
31+
| Unsafe jQuery plugin (`js/unsafe-jquery-plugin`) | Highlights potential XSS vulnerabilities in unsafely designed jQuery plugins. |
3132

3233
## Changes to existing queries
3334

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

0 commit comments

Comments
 (0)