Skip to content

Commit e354694

Browse files
author
Max Schaefer
authored
Merge pull request #273 from asger-semmle/csrf-sources
JS: add RemoteFlowSource.isThirdPartyControllable()
2 parents d2af4ab + c2a5f99 commit e354694

File tree

4 files changed

+34
-11
lines changed

4 files changed

+34
-11
lines changed

javascript/ql/src/semmle/javascript/frameworks/HTTP.qll

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,29 @@ module HTTP {
404404
* Note that this predicate is functional.
405405
*/
406406
abstract string getKind();
407+
408+
/**
409+
* Holds if this part of the request may be controlled by a third party,
410+
* that is, an agent other than the one who sent the request.
411+
*
412+
* This is true for the URL, query parameters, and request body.
413+
* These can be controlled by a malicious third party in the following scenarios:
414+
*
415+
* - The user clicks a malicious link or is otherwise redirected to a malicious URL.
416+
* - The user visits a web site that initiates a form submission or AJAX request on their behalf.
417+
*
418+
* In these cases, the request is technically sent from the user's browser, but
419+
* the user is not in direct control of the URL or POST body.
420+
*
421+
* Headers are never considered third-party controllable by this predicate, although the
422+
* third party does have some control over the the Referer and Origin headers.
423+
*/
424+
predicate isThirdPartyControllable() {
425+
exists (string kind | kind = getKind() |
426+
kind = "parameter" or
427+
kind = "url" or
428+
kind = "body")
429+
}
407430
}
408431

409432
/**

javascript/ql/src/semmle/javascript/security/dataflow/ReflectedXss.qll

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,12 @@ module ReflectedXss {
4343
}
4444
}
4545

46-
/** A source of remote user input, considered as a flow source for reflected XSS. */
47-
class RemoteFlowSourceAsSource extends Source {
48-
RemoteFlowSourceAsSource() {
49-
this instanceof RemoteFlowSource and
50-
// cookies cannot be controlled by a third-party attacker, and hence are
51-
// not relevant for reflected XSS
52-
not this.(RemoteFlowSource).getSourceType() = "Server request cookie"
46+
/** A third-party controllable request input, considered as a flow source for reflected XSS. */
47+
class ThirdPartyRequestInputAccessAsSource extends Source {
48+
ThirdPartyRequestInputAccessAsSource() {
49+
this.(HTTP::RequestInputAccess).isThirdPartyControllable()
50+
or
51+
this.(HTTP::RequestHeaderAccess).getAHeaderName() = "referer"
5352
}
5453
}
5554

javascript/ql/src/semmle/javascript/security/dataflow/RemoteFlowSources.qll

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ abstract class RemoteFlowSource extends DataFlow::Node {
1212
abstract string getSourceType();
1313
}
1414

15-
1615
/**
1716
* An access to `document.cookie`, viewed as a source of remote user input.
1817
*/

javascript/ql/src/semmle/javascript/security/dataflow/ServerSideUrlRedirect.qll

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,11 @@ module ServerSideUrlRedirect {
9090

9191
}
9292

93-
/** A source of remote user input, considered as a flow source for URL redirects. */
94-
class RemoteFlowSourceAsSource extends Source {
95-
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
93+
/** A source of third-party user input, considered as a flow source for URL redirects. */
94+
class ThirdPartyRequestInputAccessAsSource extends Source {
95+
ThirdPartyRequestInputAccessAsSource() {
96+
this.(HTTP::RequestInputAccess).isThirdPartyControllable()
97+
}
9698
}
9799

98100
/**

0 commit comments

Comments
 (0)