mirror of
https://github.com/github/codeql.git
synced 2026-04-30 19:26:02 +02:00
JS: add security query: js/request-forgery
This commit is contained in:
79
javascript/ql/src/Security/CWE-918/RequestForgery.qhelp
Normal file
79
javascript/ql/src/Security/CWE-918/RequestForgery.qhelp
Normal file
@@ -0,0 +1,79 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
|
||||
<overview>
|
||||
<p>
|
||||
|
||||
Directly incorporating user input into a remote request
|
||||
without validating the input can facilitate different kinds of request
|
||||
forgery attacks, where the attacker essentially controls the request.
|
||||
|
||||
If the vulnerable request is in server-side code, then security
|
||||
mechanisms, such as external firewalls, can be bypassed.
|
||||
|
||||
If the vulnerable request is in client-side code, then unsuspecting
|
||||
users can send malicious requests to other servers, potentially
|
||||
resulting in a DDOS attack.
|
||||
|
||||
</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
|
||||
<p>
|
||||
|
||||
To guard against request forgery, it is advisable to avoid
|
||||
putting user input directly into a remote request. If a flexible
|
||||
remote request mechanism is required, it is recommended to maintain a
|
||||
list of authorized request targets and choose from that list based on
|
||||
the user input provided.
|
||||
|
||||
</p>
|
||||
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
|
||||
<p>
|
||||
|
||||
The following example shows an HTTP request parameter
|
||||
being used directly in a URL request without validating the input,
|
||||
which facilitate an SSRF attack. The request
|
||||
<code>http.get(...)</code> is vulnerable since an attacker can choose
|
||||
the value of <code>target</code> to be anything he wants. For
|
||||
instance, the attacker can choose
|
||||
<code>"internal.example.com/#"</code> as the target, causing the URL
|
||||
used in the request to be
|
||||
<code>"https://internal.example.com/#.example.com/data"</code>.
|
||||
|
||||
</p>
|
||||
|
||||
<p>
|
||||
|
||||
A request to <code>https://internal.example.com</code> may
|
||||
be problematic if that server is not meant to be
|
||||
directly accessible from the attacker's machine.
|
||||
|
||||
</p>
|
||||
|
||||
<sample src="examples/RequestForgeryBad.js"/>
|
||||
|
||||
<p>
|
||||
|
||||
One way to remedy the problem is to use the user input to
|
||||
select a known fixed string before performing the request:
|
||||
|
||||
</p>
|
||||
|
||||
<sample src="examples/RequestForgeryGood.js"/>
|
||||
|
||||
</example>
|
||||
|
||||
<references>
|
||||
|
||||
<li>OWASP: <a href="https://www.owasp.org/index.php/Server_Side_Request_Forgery">SSRF</a></li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
18
javascript/ql/src/Security/CWE-918/RequestForgery.ql
Normal file
18
javascript/ql/src/Security/CWE-918/RequestForgery.ql
Normal file
@@ -0,0 +1,18 @@
|
||||
/**
|
||||
* @name Uncontrolled data used in remote request
|
||||
* @description Sending remote requests with user-controlled data allows for request forgery attacks.
|
||||
* @kind problem
|
||||
* @problem.severity error
|
||||
* @precision high
|
||||
* @id js/request-forgery
|
||||
* @tags security
|
||||
* external/cwe/cwe-918
|
||||
*/
|
||||
|
||||
import javascript
|
||||
import semmle.javascript.security.dataflow.RequestForgery::RequestForgery
|
||||
|
||||
from Configuration cfg, DataFlow::Node source, Sink sink, DataFlow::Node request
|
||||
where cfg.hasFlow(source, sink) and
|
||||
request = sink.getARequest()
|
||||
select request, "The $@ of this request depends on $@.", sink, sink.getKind(), source, "a user-provided value"
|
||||
@@ -0,0 +1,12 @@
|
||||
import http from 'http';
|
||||
import url from 'url';
|
||||
|
||||
var server = http.createServer(function(req, res) {
|
||||
var target = url.parse(request.url, true).query.target;
|
||||
|
||||
// BAD: `target` is controlled by the attacker
|
||||
http.get('https://' + target + ".example.com/data/", res => {
|
||||
// process request response ...
|
||||
});
|
||||
|
||||
});
|
||||
@@ -0,0 +1,19 @@
|
||||
import http from 'http';
|
||||
import url from 'url';
|
||||
|
||||
var server = http.createServer(function(req, res) {
|
||||
var target = url.parse(request.url, true).query.target;
|
||||
|
||||
var subdomain;
|
||||
if (target === 'EU') {
|
||||
subdomain = "europe"
|
||||
} else {
|
||||
subdomain = "world"
|
||||
}
|
||||
|
||||
// GOOD: `subdomain` is controlled by the server
|
||||
http.get('https://' + subdomain + ".example.com/data/", res => {
|
||||
// process request response ...
|
||||
});
|
||||
|
||||
});
|
||||
@@ -0,0 +1,87 @@
|
||||
/**
|
||||
* Provides a taint-tracking configuration for reasoning about request forgery.
|
||||
*/
|
||||
|
||||
import semmle.javascript.security.dataflow.RemoteFlowSources
|
||||
import UrlConcatenation
|
||||
|
||||
module RequestForgery {
|
||||
|
||||
/**
|
||||
* A data flow source for request forgery.
|
||||
*/
|
||||
abstract class Source extends DataFlow::Node { }
|
||||
|
||||
/**
|
||||
* A data flow sink for request forgery.
|
||||
*/
|
||||
abstract class Sink extends DataFlow::Node {
|
||||
/**
|
||||
* Gets a request that uses this sink.
|
||||
*/
|
||||
abstract DataFlow::Node getARequest();
|
||||
|
||||
/**
|
||||
* Gets the kind of this sink.
|
||||
*/
|
||||
abstract string getKind();
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
* A sanitizer for request forgery.
|
||||
*/
|
||||
abstract class Sanitizer extends DataFlow::Node { }
|
||||
|
||||
/**
|
||||
* A taint tracking configuration for request forgery.
|
||||
*/
|
||||
class Configuration extends TaintTracking::Configuration {
|
||||
Configuration() {
|
||||
this = "RequestForgery"
|
||||
}
|
||||
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
source instanceof Source
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
sink instanceof Sink
|
||||
}
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node node) {
|
||||
super.isSanitizer(node) or
|
||||
node instanceof Sanitizer
|
||||
}
|
||||
|
||||
override predicate isSanitizer(DataFlow::Node source, DataFlow::Node sink) {
|
||||
sanitizingPrefixEdge(source, sink)
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/** A source of remote user input, considered as a flow source for request forgery. */
|
||||
private class RemoteFlowSourceAsSource extends Source {
|
||||
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
|
||||
}
|
||||
|
||||
/**
|
||||
* The URL of a URL request, viewed as a sink for request forgery.
|
||||
*/
|
||||
private class UrlRequestUrlAsSink extends Sink {
|
||||
|
||||
UrlRequest request;
|
||||
|
||||
UrlRequestUrlAsSink() {
|
||||
this = request.getUrl()
|
||||
}
|
||||
|
||||
override DataFlow::Node getARequest() {
|
||||
result = request
|
||||
}
|
||||
|
||||
override string getKind() {
|
||||
result = "URL"
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user