hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

rewrite docs, improve error messages, etc

Browse Source
This commit is contained in:
Stephan Brandauer
2022-02-21 17:07:09 +01:00
parent d2335b65d5
commit 2934aa1a3a
5 changed files with 111 additions and 100 deletions
Download Patch File Download Diff File Expand all files Collapse all files

93
javascript/ql/src/Security/CWE-830/FunctionalityFromUntrustedSource.qhelp
View File

@@ -4,62 +4,67 @@
<qhelp>
<overview>
<p>
Including functionality from an external source via an <code>http</code> URL may
allow an attacker to inject malicious code via a MITM (man-in-the-middle) attack.
Including a resource from an untrusted source or using an untrusted channel may
allow an attacker to include arbitrary code in the response.
When including an external resource (eg., a <code>script</code> element or an
<code>iframe</code> element) on a page, it is important to ensure that the received
data is not malicious.
</p>
<p>
When including external resources, it is possible to verify that the origin (the server
that responds to the request) is the intended one by using an <code>https</code> URL.
This prevents a MITM (man-in-the-middle) attack where an attacker might have been able
to spoof a server response.
</p>
<p>
Even when <code>https</code> is used, an attacker might still compromise the origin server.
When using a <code>script</code> element, checking for <em>subresource integrity</em>
(checking the contents of the data received by supplying a cryptographic digest of the
expected sources to the script element) is possible. The script will only load sources
that match the digest and an attacker will be unable to modify the script even when the
server is compromised.
</p>
<p>
Subresource integrity checking is commonly recommended when importing a fixed version of
a library, eg., from a CDN (content-delivery network). Then, the fixed digest of that
version of the library can easily be added to the <code>script</code> element's
<code>integrity</code> attribute.
</p>
</overview>
<recommendation>
<p>
When including external pages or behaviour, use <code>https</code> URLs
to make sure you're getting the intended data, or users will be vulnerable
to MITM attacks.
When an <code>iframe</code> element is used to embed a page, it is important to use a
<code>https</code> URL.
</p>
<p>
When including external behaviour in iframe or script elements, using
<code>http</code> URLs is unsafe because the request sent may be intercepted
by an attacker, and malicious data may be sent back in reply.
When using a <code>script</code> element to load a script, it is important to use a
<code>https</code> URL and to consider checking subresource integrity.
</p>
<p>
Even when <code>https</code> is used, an attacker might still compromise the
server the page is receving data from.
When including scripts from a CDN (content-delivery network), it is therefore recommended
to set the integrity-attribute on the script tag to the hash of the script you're expecting
to receive.
This makes it impossible for an attacker to inject any code into the page, because the
integrity check would fail &mdash; even when the CDN is compromised.
See the reference on Subresource Integrity for more information.
</p>
</recommendation>
<example>
<p>
The following sample loads the jQuery library from the jQuery CDN without using <code>https</code>
and without checking subresource integrity.
</p>
<sample src="jquery-http-nocheck.html" />
<p>
Instead, loading jQuery from the same domain using <code>https</code> and checking
subresource integrity is recommended, as in the next sample.
</p>
<sample src="jquery-https-check.html" />
</example>
<references>
<li>
MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity">Subresource Integrity</a>
</li>
<li>
cwe.mitre.org: <a href="https://cwe.mitre.org/data/definitions/830.html">CWE 830</a>
</li>
<li>MDN: <a href="https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity">Subresource Integrity</a></li>
<li>Smashing Magazine: <a href="https://www.smashingmagazine.com/2019/04/understanding-subresource-integrity/">Understanding Subresource Integrity</a></li>
</references>
</qhelp>

90
javascript/ql/src/Security/CWE-830/FunctionalityFromUntrustedSource.ql
View File

@@ -1,10 +1,10 @@
/**
* @name Inclusion of untrusted functionality by a HTML element.
* @description Including untrusted functionality by a HTML element
* opens up for potential man-in-the-middle attacks.
* @name Inclusion of functionality from untrusted source.
* @description Including functionality from an untrusted source may allow
* an attacker to control the functionality and execute arbitrary code.
* @kind problem
* @problem.severity warning
* @security-severity 8.1
* @security-severity 6.0
* @precision high
* @id js/functionality-from-untrusted-source
* @tags security
@@ -12,33 +12,15 @@
*/
import javascript
import semmle.javascript.HTML
import semmle.javascript.dataflow.TaintTracking
module Generic {
/** A `CallNode` that creates an element of kind `name`. */
predicate isCreateElementNode(DataFlow::CallNode call, string name) {
call = DataFlow::globalVarRef("document").getAMethodCall("createElement") and
call.getArgument(0).getStringValue().toLowerCase() = name
}
/**
* A `createElement` call that creates a `<script ../>` element which never
* has its `integrity` attribute set locally.
*/
predicate isCreateScriptNodeWoIntegrityCheck(DataFlow::CallNode createCall) {
isCreateElementNode(createCall, "script") and
not exists(createCall.getAPropertyWrite("integrity"))
}
/** A location that adds a reference to an untrusted source. */
abstract class AddsUntrustedUrl extends Locatable {
/** Gets an explanation why this source is untrusted. */
abstract string getProblem();
}
/** A location that adds a reference to an untrusted source. */
abstract class AddsUntrustedUrl extends Locatable {
/** Gets an explanation why this source is untrusted. */
abstract string getProblem();
}
module StaticCreation {
/** Holds if `host` is an alias of localhost. */
bindingset[host]
predicate isLocalhostPrefix(string host) {
host.toLowerCase()
@@ -47,7 +29,7 @@ module StaticCreation {
])
}
/** A path that is vulnerable to a MITM attack. */
/** Holds if `url` is a url that is vulnerable to a MITM attack. */
bindingset[url]
predicate isUntrustedSourceUrl(string url) {
exists(string hostPath | hostPath = url.regexpCapture("(?i)http://(.*)", 1) |
@@ -55,7 +37,7 @@ module StaticCreation {
)
}
/** A path that needs an integrity check - even with https. */
/** Holds if `url` refers to a CDN that needs an integrity check - even with https. */
bindingset[url]
predicate isCdnUrlWithCheckingRequired(string url) {
// Some CDN URLs are required to have an integrity attribute. We only add CDNs to that list
@@ -68,19 +50,19 @@ module StaticCreation {
}
/** A script element that refers to untrusted content. */
class ScriptElementWithUntrustedContent extends Generic::AddsUntrustedUrl, HTML::ScriptElement {
class ScriptElementWithUntrustedContent extends AddsUntrustedUrl instanceof HTML::ScriptElement {
ScriptElementWithUntrustedContent() {
not exists(string digest | not digest = "" | this.getIntegrityDigest() = digest) and
isUntrustedSourceUrl(this.getSourcePath())
not exists(string digest | not digest = "" | super.getIntegrityDigest() = digest) and
isUntrustedSourceUrl(super.getSourcePath())
}
override string getProblem() {
result = "script elements should use an 'https:' URL and/or use the integrity attribute"
result = "HTML script element loaded using unencrypted connection."
}
}
/** A script element that refers to untrusted content. */
class CDNScriptElementWithUntrustedContent extends Generic::AddsUntrustedUrl, HTML::ScriptElement {
class CDNScriptElementWithUntrustedContent extends AddsUntrustedUrl, HTML::ScriptElement {
CDNScriptElementWithUntrustedContent() {
not exists(string digest | not digest = "" | this.getIntegrityDigest() = digest) and
isCdnUrlWithCheckingRequired(this.getSourcePath())
@@ -88,27 +70,36 @@ module StaticCreation {
override string getProblem() {
result =
"script elements that depend on this CDN should use an 'https:' URL and use the integrity attribute"
"Script loaded from content delivery network with no integrity check."
}
}
/** An iframe element that includes untrusted content. */
class IframeElementWithUntrustedContent extends HTML::IframeElement, Generic::AddsUntrustedUrl {
IframeElementWithUntrustedContent() { isUntrustedSourceUrl(this.getSourcePath()) }
class IframeElementWithUntrustedContent extends AddsUntrustedUrl instanceof HTML::IframeElement {
IframeElementWithUntrustedContent() { isUntrustedSourceUrl(super.getSourcePath()) }
override string getProblem() { result = "iframe elements should use an 'https:' URL" }
override string getProblem() { result = "HTML iframe element loaded using unencrypted connection." }
}
}
module DynamicCreation {
import DataFlow::TypeTracker
/** Holds if `call` creates a tag of kind `name`. */
predicate isCreateElementNode(DataFlow::CallNode call, string name) {
call = DataFlow::globalVarRef("document").getAMethodCall("createElement") and
call.getArgument(0).getStringValue().toLowerCase() = name
}
predicate isUnsafeSourceLiteral(DataFlow::Node source) {
exists(StringLiteral s | source = s.flow() | s.getValue().regexpMatch("(?i)http:.*"))
/**
* Holds if `createCall` creates a `<script ../>` element which never
* has its `integrity` attribute set locally.
*/
predicate isCreateScriptNodeWoIntegrityCheck(DataFlow::CallNode createCall) {
isCreateElementNode(createCall, "script") and
not exists(createCall.getAPropertyWrite("integrity"))
}
DataFlow::Node urlTrackedFromUnsafeSourceLiteral(DataFlow::TypeTracker t) {
t.start() and isUnsafeSourceLiteral(result)
t.start() and result.getStringValue().regexpMatch("(?i)http:.*")
or
exists(DataFlow::TypeTracker t2, DataFlow::Node prev |
prev = urlTrackedFromUnsafeSourceLiteral(t2)
@@ -137,16 +128,16 @@ module DynamicCreation {
predicate isAssignedToSrcAttribute(string name, DataFlow::Node sink) {
exists(DataFlow::CallNode createElementCall |
name = "script" and
Generic::isCreateScriptNodeWoIntegrityCheck(createElementCall) and
isCreateScriptNodeWoIntegrityCheck(createElementCall) and
sink = createElementCall.getAPropertyWrite("src").getRhs()
or
name = "iframe" and
Generic::isCreateElementNode(createElementCall, "iframe") and
isCreateElementNode(createElementCall, "iframe") and
sink = createElementCall.getAPropertyWrite("src").getRhs()
)
}
class IframeOrScriptSrcAssignment extends Expr, Generic::AddsUntrustedUrl {
class IframeOrScriptSrcAssignment extends AddsUntrustedUrl {
string name;
IframeOrScriptSrcAssignment() {
@@ -157,13 +148,10 @@ module DynamicCreation {
}
override string getProblem() {
name = "script" and
result = "script elements should use an 'https:' URL and/or use the integrity attribute"
or
name = "iframe" and result = "iframe elements should use an 'https:' URL"
result = "HTML " + name + " element loaded using unencrypted connection."
}
}
}
from Generic::AddsUntrustedUrl s
select s, "HTML-element uses untrusted content (" + s.getProblem() + ")"
from AddsUntrustedUrl s
select s, s.getProblem()

9
javascript/ql/src/Security/CWE-830/jquery-http-nocheck.html Normal file
View File

@@ -0,0 +1,9 @@
<html>
<head>
<title>jQuery demo</title>
<script src="http://code.jquery.com/jquery-3.6.0.slim.min.js" crossorigin="anonymous"></script>
</head>
<body>
...
</body>
</html>

9
javascript/ql/src/Security/CWE-830/jquery-https-check.html Normal file
View File

@@ -0,0 +1,9 @@
<html>
<head>
<title>jQuery demo</title>
<script src="https://code.jquery.com/jquery-3.6.0.slim.min.js" integrity="sha256-u7e5khyithlIdTpu22PHhENmPcRdFiHRjhAuHcs05RI=" crossorigin="anonymous"></script>
</head>
<body>
...
</body>
</html>