Skip to content

Commit a664e02

Browse files
author
Stephan Brandauer
authored
Merge pull request #8014 from kaeluka/js/functionality-from-untrusted-source
JS: Functionality from untrusted sources query (CWE-830)
2 parents f011bbc + 1ed71e1 commit a664e02

10 files changed

+366
-0
lines changed

‎javascript/ql/lib/semmle/javascript/HTML.qll

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,24 @@ module HTML {
173173
DocumentElement() { getName() = "html" }
174174
}
175175

176+
/**
177+
* An HTML `<iframe>` element.
178+
*
179+
* Example:
180+
*
181+
* ```
182+
* <iframe src="https://test.local/somepage.html"></iframe>
183+
* ```
184+
*/
185+
class IframeElement extends Element {
186+
IframeElement() { getName() = "iframe" }
187+
188+
/**
189+
* Gets the value of the `src` attribute.
190+
*/
191+
string getSourcePath() { result = getAttributeByName("src").getValue() }
192+
}
193+
176194
/**
177195
* An HTML `<script>` element.
178196
*
@@ -207,6 +225,11 @@ module HTML {
207225
*/
208226
string getSourcePath() { result = getAttributeByName("src").getValue() }
209227

228+
/**
229+
* Gets the value of the `integrity` attribute.
230+
*/
231+
string getIntegrityDigest() { result = getAttributeByName("integrity").getValue() }
232+
210233
/**
211234
* Gets the folder relative to which the `src` attribute is resolved.
212235
*/

‎javascript/ql/src/Security/CWE-830/FunctionalityFromUntrustedSource.qhelp

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>
7+
Including a resource from an untrusted source or using an untrusted channel may
8+
allow an attacker to include arbitrary code in the response.
9+
When including an external resource (for example, a <code>script</code> element or an
10+
<code>iframe</code> element) on a page, it is important to ensure that the received
11+
data is not malicious.
12+
</p>
13+
14+
<p>
15+
When including external resources, it is possible to verify that the responding server
16+
is the intended one by using an <code>https</code> URL.
17+
This prevents a MITM (man-in-the-middle) attack where an attacker might have been able
18+
to spoof a server response.
19+
</p>
20+
21+
<p>
22+
Even when <code>https</code> is used, an attacker might still compromise the server.
23+
When you use a <code>script</code> element, you can check for subresource integrity -
24+
that is, you can check the contents of the data received by supplying a cryptographic
25+
digest of the expected sources to the <code>script</code> element. The script will only
26+
load sources that match the digest and an attacker will be unable to modify the script
27+
even when the server is compromised.
28+
</p>
29+
30+
<p>
31+
Subresource integrity checking is commonly recommended when importing a fixed version of
32+
a library - for example, from a CDN (content-delivery network). Then, the fixed digest
33+
of that version of the library can easily be added to the <code>script</code> element's
34+
<code>integrity</code> attribute.
35+
</p>
36+
</overview>
37+
38+
<recommendation>
39+
<p>
40+
When an <code>iframe</code> element is used to embed a page, it is important to use an
41+
<code>https</code> URL.
42+
</p>
43+
44+
<p>
45+
When using a <code>script</code> element to load a script, it is important to use an
46+
<code>https</code> URL and to consider checking subresource integrity.
47+
</p>
48+
</recommendation>
49+
50+
<example>
51+
<p>
52+
The following example loads the jQuery library from the jQuery CDN without using <code>https</code>
53+
and without checking subresource integrity.
54+
</p>
55+
56+
<sample src="jquery-http-nocheck.html" />
57+
58+
<p>
59+
Instead, loading jQuery from the same domain using <code>https</code> and checking
60+
subresource integrity is recommended, as in the next example.
61+
</p>
62+
63+
<sample src="jquery-https-check.html" />
64+
</example>
65+
66+
<references>
67+
<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity">Subresource Integrity</a></li>
68+
<li>Smashing Magazine: <a href="https://www.smashingmagazine.com/2019/04/understanding-subresource-integrity/">Understanding Subresource Integrity</a></li>
69+
</references>
70+
</qhelp>

‎javascript/ql/src/Security/CWE-830/FunctionalityFromUntrustedSource.ql

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
/**
2+
* @name Inclusion of functionality from an untrusted source
3+
* @description Including functionality from an untrusted source may allow
4+
* an attacker to control the functionality and execute arbitrary code.
5+
* @kind problem
6+
* @problem.severity warning
7+
* @security-severity 6.0
8+
* @precision high
9+
* @id js/functionality-from-untrusted-source
10+
* @tags security
11+
* external/cwe/cwe-830
12+
*/
13+
14+
import javascript
15+
16+
/** A location that adds a reference to an untrusted source. */
17+
abstract class AddsUntrustedUrl extends Locatable {
18+
/** Gets an explanation why this source is untrusted. */
19+
abstract string getProblem();
20+
}
21+
22+
module StaticCreation {
23+
/** Holds if `host` is an alias of localhost. */
24+
bindingset[host]
25+
predicate isLocalhostPrefix(string host) {
26+
host.toLowerCase()
27+
.regexpMatch([
28+
"(?i)localhost(:[0-9]+)?/.*", "127.0.0.1(:[0-9]+)?/.*", "::1/.*", "\\[::1\\]:[0-9]+/.*"
29+
])
30+
}
31+
32+
/** Holds if `url` is a url that is vulnerable to a MITM attack. */
33+
bindingset[url]
34+
predicate isUntrustedSourceUrl(string url) {
35+
exists(string hostPath | hostPath = url.regexpCapture("(?i)http://(.*)", 1) |
36+
not isLocalhostPrefix(hostPath)
37+
)
38+
}
39+
40+
/** Holds if `url` refers to a CDN that needs an integrity check - even with https. */
41+
bindingset[url]
42+
predicate isCdnUrlWithCheckingRequired(string url) {
43+
// Some CDN URLs are required to have an integrity attribute. We only add CDNs to that list
44+
// that recommend integrity-checking.
45+
url.regexpMatch("(?i)^https?://" +
46+
[
47+
"code\\.jquery\\.com", //
48+
"cdnjs\\.cloudflare\\.com", //
49+
"cdnjs\\.com" //
50+
] + "/.*\\.js$")
51+
}
52+
53+
/** A script element that refers to untrusted content. */
54+
class ScriptElementWithUntrustedContent extends AddsUntrustedUrl instanceof HTML::ScriptElement {
55+
ScriptElementWithUntrustedContent() {
56+
not exists(string digest | not digest = "" | super.getIntegrityDigest() = digest) and
57+
isUntrustedSourceUrl(super.getSourcePath())
58+
}
59+
60+
override string getProblem() { result = "Script loaded using unencrypted connection." }
61+
}
62+
63+
/** A script element that refers to untrusted content. */
64+
class CDNScriptElementWithUntrustedContent extends AddsUntrustedUrl, HTML::ScriptElement {
65+
CDNScriptElementWithUntrustedContent() {
66+
not exists(string digest | not digest = "" | this.getIntegrityDigest() = digest) and
67+
isCdnUrlWithCheckingRequired(this.getSourcePath())
68+
}
69+
70+
override string getProblem() {
71+
result = "Script loaded from content delivery network with no integrity check."
72+
}
73+
}
74+
75+
/** An iframe element that includes untrusted content. */
76+
class IframeElementWithUntrustedContent extends AddsUntrustedUrl instanceof HTML::IframeElement {
77+
IframeElementWithUntrustedContent() { isUntrustedSourceUrl(super.getSourcePath()) }
78+
79+
override string getProblem() { result = "Iframe loaded using unencrypted connection." }
80+
}
81+
}
82+
83+
module DynamicCreation {
84+
/** Holds if `call` creates a tag of kind `name`. */
85+
predicate isCreateElementNode(DataFlow::CallNode call, string name) {
86+
call = DataFlow::globalVarRef("document").getAMethodCall("createElement") and
87+
call.getArgument(0).getStringValue().toLowerCase() = name
88+
}
89+
90+
DataFlow::Node getAttributeAssignmentRhs(DataFlow::CallNode createCall, string name) {
91+
result = createCall.getAPropertyWrite(name).getRhs()
92+
or
93+
exists(DataFlow::InvokeNode inv | inv = createCall.getAMemberInvocation("setAttribute") |
94+
inv.getArgument(0).getStringValue() = name and
95+
result = inv.getArgument(1)
96+
)
97+
}
98+
99+
/**
100+
* Holds if `createCall` creates a `<script ../>` element which never
101+
* has its `integrity` attribute set locally.
102+
*/
103+
predicate isCreateScriptNodeWoIntegrityCheck(DataFlow::CallNode createCall) {
104+
isCreateElementNode(createCall, "script") and
105+
not exists(getAttributeAssignmentRhs(createCall, "integrity"))
106+
}
107+
108+
DataFlow::Node urlTrackedFromUnsafeSourceLiteral(DataFlow::TypeTracker t) {
109+
t.start() and result.getStringValue().regexpMatch("(?i)http:.*")
110+
or
111+
exists(DataFlow::TypeTracker t2, DataFlow::Node prev |
112+
prev = urlTrackedFromUnsafeSourceLiteral(t2)
113+
|
114+
not exists(string httpsUrl | httpsUrl.toLowerCase() = "https:" + any(string rest) |
115+
// when the result may have a string value starting with https,
116+
// we're most likely with an assignment like:
117+
// e.src = ('https:' == document.location.protocol ? 'https://ssl' : 'http://www') + '.google-analytics.com/ga.js'
118+
// these assignments, we don't want to fix - once the browser is using http,
119+
// MITM attacks are possible anyway.
120+
result.mayHaveStringValue(httpsUrl)
121+
) and
122+
(
123+
t2 = t.smallstep(prev, result)
124+
or
125+
TaintTracking::sharedTaintStep(prev, result) and
126+
t = t2
127+
)
128+
)
129+
}
130+
131+
DataFlow::Node urlTrackedFromUnsafeSourceLiteral() {
132+
result = urlTrackedFromUnsafeSourceLiteral(DataFlow::TypeTracker::end())
133+
}
134+
135+
/** Holds if `sink` is assigned to the attribute `name` of any HTML element. */
136+
predicate isAssignedToSrcAttribute(string name, DataFlow::Node sink) {
137+
exists(DataFlow::CallNode createElementCall |
138+
sink = getAttributeAssignmentRhs(createElementCall, "src") and
139+
(
140+
name = "script" and
141+
isCreateScriptNodeWoIntegrityCheck(createElementCall)
142+
or
143+
name = "iframe" and
144+
isCreateElementNode(createElementCall, "iframe")
145+
)
146+
)
147+
}
148+
149+
class IframeOrScriptSrcAssignment extends AddsUntrustedUrl {
150+
string name;
151+
152+
IframeOrScriptSrcAssignment() {
153+
name = ["script", "iframe"] and
154+
exists(DataFlow::Node n | n.asExpr() = this |
155+
isAssignedToSrcAttribute(name, n) and
156+
n = urlTrackedFromUnsafeSourceLiteral()
157+
)
158+
}
159+
160+
override string getProblem() {
161+
name = "script" and result = "Script loaded using unencrypted connection."
162+
or
163+
name = "iframe" and result = "Iframe loaded using unencrypted connection."
164+
}
165+
}
166+
}
167+
168+
from AddsUntrustedUrl s
169+
select s, s.getProblem()

‎javascript/ql/src/Security/CWE-830/jquery-http-nocheck.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<html>
2+
<head>
3+
<title>jQuery demo</title>
4+
<script src="http://code.jquery.com/jquery-3.6.0.slim.min.js" crossorigin="anonymous"></script>
5+
</head>
6+
<body>
7+
...
8+
</body>
9+
</html>

‎javascript/ql/src/Security/CWE-830/jquery-https-check.html

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<html>
2+
<head>
3+
<title>jQuery demo</title>
4+
<script src="https://code.jquery.com/jquery-3.6.0.slim.min.js" integrity="sha256-u7e5khyithlIdTpu22PHhENmPcRdFiHRjhAuHcs05RI=" crossorigin="anonymous"></script>
5+
</head>
6+
<body>
7+
...
8+
</body>
9+
</html>

‎javascript/ql/src/change-notes/2022-02-14-functionality-from-untrusted-source.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
category: newQuery
3+
---
4+
* A new query, `js/functionality-from-untrusted-source`, has been added to the query suite. It finds DOM elements
5+
that load functionality from untrusted sources, like `script` or `iframe` elements using `http` links.
6+
The query is run by default.

‎javascript/ql/test/query-tests/Security/CWE-830/DynamicCreationOfUntrustedSourceUse.html

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<html>
2+
<head>
3+
<script type="text/javascript">
4+
(function() {
5+
// OK (we accept this, as a http document location is vulnerable anyway)
6+
var scrpt = document.createElement('script');
7+
scrpt.type = 'text/javascript';
8+
scrpt.src = ('https:' == document.location.protocol ? 'https://ssl' : 'http://www') + '.cdn.local/ga.js';
9+
10+
// OK (integrity digest present)
11+
var scrpt2 = document.createElement('script');
12+
scrpt2.type = 'text/javascript';
13+
scrpt2.src = 'http://www.cdn.local/ga.js';
14+
scrpt2.integrity = 'sha256-h0UuK3mE9taiYlB5u9vT9A0s/XDgkfVd+F4VhN/sky=';
15+
16+
// NOT OK (http + ternary)
17+
var scrpt3 = document.createElement('script');
18+
scrpt3.type = 'text/javascript';
19+
scrpt3.src = ('https:' == document.location.protocol ? 'http://unsafe' : 'http://also-unsafe') + '.cdn.local/ga.js';
20+
21+
// NOT OK (http URL)
22+
var ifrm = document.createElement('iframe');
23+
ifrm.src = 'http://www.example.com/';
24+
25+
// OK (https URL)
26+
var ifrm2 = document.createElement('iframe');
27+
ifrm2.src = 'https://www.example.com/';
28+
29+
// NOT OK (http URL tracked through calls)
30+
function getUrl(version) {
31+
return 'http://www.cdn.local/'+version+'/ga.js';
32+
}
33+
var ifrm3 = document.createElement('iframe');
34+
ifrm3.src = getUrl('v123');
35+
36+
// NOT OK (assignment of bad URL using setAttribute)
37+
var ifrm4 = document.createElement('iframe');
38+
ifrm4.setAttribute('src', 'http://www.example.local/page.html');
39+
40+
// OK (bad URL, but the attribute is not `src`)
41+
var ifrm5 = document.createElement('iframe');
42+
ifrm5.setAttribute('data-src', 'http://www.example.local/page.html');
43+
})();
44+
</script>
45+
</head>
46+
<body>
47+
hello
48+
</body>
49+
</html>

‎javascript/ql/test/query-tests/Security/CWE-830/FunctionalityFromUntrustedSource.expected

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
| DynamicCreationOfUntrustedSourceUse.html:19:28:19:129 | ('https ... /ga.js' | Script loaded using unencrypted connection. |
2+
| DynamicCreationOfUntrustedSourceUse.html:23:26:23:50 | 'http:/ ... e.com/' | Iframe loaded using unencrypted connection. |
3+
| DynamicCreationOfUntrustedSourceUse.html:34:27:34:40 | getUrl('v123') | Iframe loaded using unencrypted connection. |
4+
| DynamicCreationOfUntrustedSourceUse.html:38:41:38:76 | 'http:/ ... e.html' | Iframe loaded using unencrypted connection. |
5+
| StaticCreationOfUntrustedSourceUse.html:6:9:6:56 | <script>...</> | Script loaded using unencrypted connection. |
6+
| StaticCreationOfUntrustedSourceUse.html:9:9:9:58 | <iframe>...</> | Iframe loaded using unencrypted connection. |
7+
| StaticCreationOfUntrustedSourceUse.html:21:9:21:155 | <script>...</> | Script loaded from content delivery network with no integrity check. |

‎javascript/ql/test/query-tests/Security/CWE-830/FunctionalityFromUntrustedSource.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE-830/FunctionalityFromUntrustedSource.ql

‎javascript/ql/test/query-tests/Security/CWE-830/StaticCreationOfUntrustedSourceUse.html

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE html>
2+
<html>
3+
<head>
4+
</head>
5+
<body>
6+
<script src="http://test.local/foo.js"></script>> <!-- NOT OK -->
7+
<script src="http://test.local/foo.js" integrity="some-integrity-hash"></script>> <!-- OK (integrity digest present) -->
8+
<script src="https://test.local/bar.js"></script>> <!-- OK (https) -->
9+
<iframe src="http://test.local/foo.html"></iframe> <!-- NOT OK -->
10+
<iframe src="https://test.local/foo.html"></iframe> <!-- OK (https) -->
11+
<iframe src="//test.local/foo.html"></iframe> <!-- OK (protocol-relative url is allowed as a http url of
12+
the page is vulnerable in the first place) -->
13+
<iframe src="http://::1/foo.html"></iframe> <!-- OK (localhost) -->
14+
<iframe src="http://[::1]:80/foo.html"></iframe> <!-- OK (localhost) -->
15+
<iframe src="http://127.0.0.1:444/foo.html"></iframe> <!-- OK (localhost) -->
16+
17+
<!-- Some CDNs recommend using the integrity attribute — for those, we demand it even with https links -->
18+
<!-- OK (digest present) -->
19+
<script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.8.2/angular.min.js" integrity="sha512-7oYXeK0OxTFxndh0erL8FsjGvrl2VMDor6fVqzlLGfwOQQqTbYsGPv4ZZ15QHfSk80doyaM0ZJdvkyDcVO7KFA==" crossorigin="anonymous" referrerpolicy="no-referrer"></script>
20+
<!-- NOT OK (digest missing) -->
21+
<script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.8.2/angular.min.js" crossorigin="anonymous" referrerpolicy="no-referrer"></script>
22+
</body>
23+
</html>

0 commit comments

Comments
 (0)