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

JS: Add ClientSideRemoteFlowSource

Browse Source
This commit is contained in:
Asger Feldthaus
2021-01-19 16:45:53 +00:00
parent aa360c0378
commit 2e57a7d3e9
26 changed files with 659 additions and 793 deletions
Download Patch File Download Diff File Expand all files Collapse all files

32
javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll
View File

@@ -239,6 +239,35 @@ module TaintTracking {
}
}
/** Gets a data flow node referring to the client side URL. */
private DataFlow::SourceNode clientSideUrlRef(DataFlow::TypeTracker t) {
t.start() and
result.(ClientSideRemoteFlowSource).getKind().isUrl()
or
exists(DataFlow::TypeTracker t2 | result = clientSideUrlRef(t2).track(t2, t))
}
/** Gets a data flow node referring to the client side URL. */
private DataFlow::SourceNode clientSideUrlRef() {
result = clientSideUrlRef(DataFlow::TypeTracker::end())
}
/**
* Holds if `read` reads a property of the client-side URL, which is not tainted.
* In this case, the read is excluded from the default set of taint steps.
*/
private predicate isSafeClientSideUrlProperty(DataFlow::PropRead read) {
// Block all properties of client-side URLs, as .hash and .search are considered sources of their own
read = clientSideUrlRef().getAPropertyRead()
or
exists(StringSplitCall c |
c.getBaseString().getALocalSource() =
[DOM::locationRef(), DOM::locationRef().getAPropertyRead("href")] and
c.getSeparator() = "?" and
read = c.getAPropertyRead("0")
)
}
/**
* Holds if there is taint propagation through the heap from `pred` to `succ`.
*/
@@ -264,7 +293,8 @@ module TaintTracking {
or
// reading from a tainted object yields a tainted result
succ.(DataFlow::PropRead).getBase() = pred and
not AccessPath::DominatingPaths::hasDominatingWrite(succ)
not AccessPath::DominatingPaths::hasDominatingWrite(succ) and
not isSafeClientSideUrlProperty(succ)
or
// iterating over a tainted iterator taints the loop variable
exists(ForOfStmt fos |

57
javascript/ql/src/semmle/javascript/frameworks/Angular2.qll
View File

@@ -77,46 +77,61 @@ module Angular2 {
}
/** Gets a reference to a `ParamMap` object, usually containing values from the URL. */
DataFlow::SourceNode paramMap() {
result.hasUnderlyingType("@angular/router", "ParamMap")
private DataFlow::SourceNode paramMap(ClientSideRemoteFlowKind kind) {
result.hasUnderlyingType("@angular/router", "ParamMap") and kind.isPath()
or
result = activatedRouteProp(["paramMap", "queryParamMap"])
result = activatedRouteProp("paramMap") and kind.isPath()
or
result = urlSegment().getAPropertyRead("parameterMap")
result = activatedRouteProp("queryParamMap") and kind.isQuery()
or
result = urlSegment().getAPropertyRead("parameterMap") and kind.isPath()
}
/** Gets a reference to a `ParamMap` object, usually containing values from the URL. */
DataFlow::SourceNode paramMap() { result = paramMap(_) }
/** Gets a reference to a `Params` object, usually containing values from the URL. */
private DataFlow::SourceNode paramDictionaryObject(ClientSideRemoteFlowKind kind) {
result.hasUnderlyingType("@angular/router", "Params") and
kind.isPath() and
not result instanceof DataFlow::ObjectLiteralNode // ignore object literals found by contextual typing
or
result = activatedRouteProp("params") and kind.isPath()
or
result = activatedRouteProp("queryParams") and kind.isQuery()
or
result = paramMap(kind).getAPropertyRead("params")
or
result = urlSegment().getAPropertyRead("parameters") and kind.isPath()
}
/** Gets a reference to a `Params` object, usually containing values from the URL. */
DataFlow::SourceNode paramDictionaryObject() {
result.hasUnderlyingType("@angular/router", "Params") and
not result instanceof DataFlow::ObjectLiteralNode // ignore object literals found by contextual typing
or
result = activatedRouteProp(["params", "queryParams"])
or
result = paramMap().getAPropertyRead("params")
or
result = urlSegment().getAPropertyRead("parameters")
}
DataFlow::SourceNode paramDictionaryObject() { result = paramDictionaryObject(_) }
/**
* A value from `@angular/router` derived from the URL.
*/
class AngularSource extends RemoteFlowSource {
class AngularSource extends ClientSideRemoteFlowSource {
ClientSideRemoteFlowKind kind;
AngularSource() {
this = paramMap().getAMethodCall(["get", "getAll"])
this = paramMap(kind).getAMethodCall(["get", "getAll"])
or
this = paramDictionaryObject()
this = paramDictionaryObject(kind)
or
this = activatedRouteProp("fragment")
this = activatedRouteProp("fragment") and kind.isFragment()
or
this = urlSegment().getAPropertyRead("path")
this = urlSegment().getAPropertyRead("path") and kind.isPath()
or
// Note that Router.url and RouterStateSnapshot.url are strings, not UrlSegment[]
this = router().getAPropertyRead("url")
this = router().getAPropertyRead("url") and kind.isUrl()
or
this = routerStateSnapshot().getAPropertyRead("url")
this = routerStateSnapshot().getAPropertyRead("url") and kind.isUrl()
}
override string getSourceType() { result = "Angular route parameter" }
override ClientSideRemoteFlowKind getKind() { result = kind }
}
/** Gets a reference to a `DomSanitizer` object. */

4
javascript/ql/src/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll
View File

@@ -636,7 +636,7 @@ private class LocationFlowSource extends RemoteFlowSource {
*
* See https://docs.angularjs.org/api/ngRoute/service/$routeParams for more details.
*/
private class RouteParamSource extends RemoteFlowSource {
private class RouteParamSource extends ClientSideRemoteFlowSource {
RouteParamSource() {
exists(ServiceReference service |
service.getName() = "$routeParams" and
@@ -645,6 +645,8 @@ private class RouteParamSource extends RemoteFlowSource {
}
override string getSourceType() { result = "$routeParams" }
override ClientSideRemoteFlowKind getKind() { result.isPath() }
}
/**

16
javascript/ql/src/semmle/javascript/frameworks/React.qll
View File

@@ -686,14 +686,24 @@ private DataFlow::SourceNode reactRouterDom() {
result = DataFlow::moduleImport("react-router-dom")
}
private class ReactRouterSource extends RemoteFlowSource {
private class ReactRouterSource extends ClientSideRemoteFlowSource {
ClientSideRemoteFlowKind kind;
ReactRouterSource() {
this = reactRouterDom().getAMemberCall("useParams")
this = reactRouterDom().getAMemberCall("useParams") and kind.isPath()
or
this = reactRouterDom().getAMemberCall("useRouteMatch").getAPropertyRead(["params", "url"])
exists(string prop |
this = reactRouterDom().getAMemberCall("useRouteMatch").getAPropertyRead(prop)
|
prop = "params" and kind.isPath()
or
prop = "url" and kind.isUrl()
)
}
override string getSourceType() { result = "react-router path parameters" }
override ClientSideRemoteFlowKind getKind() { result = kind }
}
/**

5
javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll
View File

@@ -25,11 +25,8 @@ module ClientSideUrlRedirect {
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "ClientSideUrlRedirect" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel lbl) {
source = DOM::locationSource() and
lbl instanceof DocumentUrl
source.(Source).getAFlowLabel() = lbl
}
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }

46
javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll
View File

@@ -13,7 +13,10 @@ module ClientSideUrlRedirect {
/**
* A data flow source for unvalidated URL redirect vulnerabilities.
*/
abstract class Source extends DataFlow::Node { }
abstract class Source extends DataFlow::Node {
/** Gets a flow label to associate with this source. */
DataFlow::FlowLabel getAFlowLabel() { result.isTaint() }
}
/**
* A data flow sink for unvalidated URL redirect vulnerabilities.
@@ -35,22 +38,32 @@ module ClientSideUrlRedirect {
/** A source of remote user input, considered as a flow source for unvalidated URL redirects. */
class RemoteFlowSourceAsSource extends Source {
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
RemoteFlowSourceAsSource() {
this instanceof RemoteFlowSource and
not this.(ClientSideRemoteFlowSource).getKind().isPath()
}
override DataFlow::FlowLabel getAFlowLabel() {
if this.(ClientSideRemoteFlowSource).getKind().isUrl()
then result instanceof DocumentUrl
else result.isTaint()
}
}
/**
* Holds if `queryAccess` is an expression that may access the query string
* of a URL that flows into `nd` (that is, the part after the `?`).
* DEPRECATED. Can usually be replaced with `untrustedUrlSubstring`.
* Query accesses via `location.hash` or `location.search` are now independent
* `RemoteFlowSource` instances, and only substrings of `location` need to be handled via steps.
*/
predicate queryAccess(DataFlow::Node nd, DataFlow::Node queryAccess) {
exists(string propertyName |
queryAccess.asExpr().(PropAccess).accesses(nd.asExpr(), propertyName)
|
propertyName = "search" or propertyName = "hash"
)
or
deprecated predicate queryAccess = untrustedUrlSubstring/2;
/**
* Holds if `substring` refers to a substring of `base` which is considered untrusted
* when `base` is the current URL.
*/
predicate untrustedUrlSubstring(DataFlow::Node base, DataFlow::Node substring) {
exists(MethodCallExpr mce, string methodName |
mce = queryAccess.asExpr() and mce.calls(nd.asExpr(), methodName)
mce = substring.asExpr() and mce.calls(base.asExpr(), methodName)
|
methodName = "split" and
// exclude all splits where only the prefix is accessed, which is safe for url-redirects.
@@ -63,17 +76,12 @@ module ClientSideUrlRedirect {
)
or
exists(MethodCallExpr mce |
queryAccess.asExpr() = mce and
substring.asExpr() = mce and
mce = any(DataFlow::RegExpCreationNode re).getAMethodCall("exec").asExpr() and
nd.asExpr() = mce.getArgument(0)
base.asExpr() = mce.getArgument(0)
)
}
/**
* A sanitizer that reads the first part a location split by "?", e.g. `location.href.split('?')[0]`.
*/
class QueryPrefixSanitizer extends Sanitizer, DomBasedXss::QueryPrefixSanitizer { }
/**
* A sink which is used to set the window location.
*/

1
javascript/ql/src/semmle/javascript/security/dataflow/CodeInjection.qll
View File

@@ -24,7 +24,6 @@ module CodeInjection {
override predicate isSanitizer(DataFlow::Node node) {
super.isSanitizer(node) or
isSafeLocationProperty(node.asExpr()) or
node instanceof Sanitizer
}

7
javascript/ql/src/semmle/javascript/security/dataflow/CodeInjectionCustomizations.qll
View File

@@ -32,13 +32,6 @@ module CodeInjection {
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
}
/**
* An access to a property that may hold (parts of) the document URL.
*/
class LocationSource extends Source {
LocationSource() { this = DOM::locationSource() }
}
/**
* An expression which may be interpreted as an AngularJS expression.
*/

5
javascript/ql/src/semmle/javascript/security/dataflow/CommandInjectionCustomizations.qll
View File

@@ -28,7 +28,10 @@ module CommandInjection {
/** A source of remote user input, considered as a flow source for command injection. */
class RemoteFlowSourceAsSource extends Source {
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
RemoteFlowSourceAsSource() {
this instanceof RemoteFlowSource and
not this instanceof ClientSideRemoteFlowSource
}
override string getSourceType() { result = "a user-provided value" }
}

5
javascript/ql/src/semmle/javascript/security/dataflow/CorsMisconfigurationForCredentialsCustomizations.qll
View File

@@ -29,7 +29,10 @@ module CorsMisconfigurationForCredentials {
/** A source of remote user input, considered as a flow source for CORS misconfiguration. */
class RemoteFlowSourceAsSource extends Source {
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
RemoteFlowSourceAsSource() {
this instanceof RemoteFlowSource and
not this instanceof ClientSideRemoteFlowSource
}
}
/**

29
javascript/ql/src/semmle/javascript/security/dataflow/DOM.qll
View File

@@ -40,11 +40,14 @@ predicate isDocument(Expr e) { DOM::documentRef().flowsToExpr(e) }
predicate isDocumentURL(Expr e) { e.flow() = DOM::locationSource() }
/**
* DEPRECATED. In most cases, a sanitizer based on this predicate can be removed, as
* taint tracking no longer step through the properties of the location object by default.
*
* Holds if `pacc` accesses a part of `document.location` that is
* not considered user-controlled, that is, anything except
* `href`, `hash` and `search`.
*/
predicate isSafeLocationProperty(PropAccess pacc) {
deprecated predicate isSafeLocationProperty(PropAccess pacc) {
exists(string prop | pacc = DOM::locationRef().getAPropertyRead(prop).asExpr() |
prop != "href" and prop != "hash" and prop != "search"
)
@@ -224,7 +227,7 @@ private class PostMessageEventParameter extends RemoteFlowSource {
* An access to `window.name`, which can be controlled by the opener of the window,
* even if the window is opened from a foreign domain.
*/
private class WindowNameAccess extends RemoteFlowSource {
private class WindowNameAccess extends ClientSideRemoteFlowSource {
pragma[nomagic, noinline]
WindowNameAccess() {
this = DataFlow::globalObjectRef().getAPropertyRead("name")
@@ -238,4 +241,26 @@ private class WindowNameAccess extends RemoteFlowSource {
}
override string getSourceType() { result = "Window name" }
override ClientSideRemoteFlowKind getKind() { result.isWindowName() }
}
private class WindowLocationFlowSource extends ClientSideRemoteFlowSource {
ClientSideRemoteFlowKind kind;
WindowLocationFlowSource() {
this = DOM::locationSource() and kind.isUrl()
or
// Add separate sources for the properties of window.location as they are excluded
// from the default taint steps.
this = DOM::locationRef().getAPropertyRead("hash") and kind.isFragment()
or
this = DOM::locationRef().getAPropertyRead("search") and kind.isQuery()
or
this = DOM::locationRef().getAPropertyRead("href") and kind.isUrl()
}
override string getSourceType() { result = "Window location" }
override ClientSideRemoteFlowKind getKind() { result = kind }
}

2
javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXss.qll
View File

@@ -53,7 +53,7 @@ module DomBasedXss {
override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
// Reuse any source not derived from location
source instanceof Source and
not source = DOM::locationRef() and
not source = [DOM::locationRef(), DOM::locationRef().getAPropertyRead()] and
label.isTaint()
or
source = DOM::locationSource() and

7
javascript/ql/src/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll
View File

@@ -12,11 +12,4 @@ module DomBasedXss {
class RemoteFlowSourceAsSource extends Source {
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
}
/**
* An access of the URL of this page, or of the referrer to this page.
*/
class LocationSource extends Source {
LocationSource() { this = DOM::locationSource() }
}
}

8
javascript/ql/src/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataCustomizations.qll
View File

@@ -59,14 +59,6 @@ module ExternalAPIUsedWithUntrustedData {
RemoteFlowAsSource() { this instanceof RemoteFlowSource }
}
private class LocationSource extends Source {
LocationSource() {
this = DOM::locationRef().getAPropertyRead(["hash", "search"])
or
this = DOM::locationSource()
}
}
/**
* A package name whose entire API is considered "safe" for the purpose of this query.
*/

4
javascript/ql/src/semmle/javascript/security/dataflow/LogInjection.qll
View File

@@ -40,7 +40,9 @@ module LogInjection {
* A source of remote user controlled input.
*/
class RemoteSource extends Source {
RemoteSource() { this instanceof RemoteFlowSource }
RemoteSource() {
this instanceof RemoteFlowSource and not this instanceof ClientSideRemoteFlowSource
}
}
/**

6
javascript/ql/src/semmle/javascript/security/dataflow/PrototypePollutingAssignmentCustomizations.qll
View File

@@ -51,10 +51,6 @@ module PrototypePollutingAssignment {
/** A remote flow source or location.{hash,search} as a taint source. */
private class DefaultSource extends Source {
DefaultSource() {
this instanceof RemoteFlowSource
or
this = DOM::locationRef().getAPropertyRead(["hash", "search"])
}
DefaultSource() { this instanceof RemoteFlowSource }
}
}

53
javascript/ql/src/semmle/javascript/security/dataflow/RemoteFlowSources.qll
View File

@@ -10,7 +10,7 @@ private import semmle.javascript.internal.CachedStages
/** A data flow source of remote user input. */
cached
abstract class RemoteFlowSource extends DataFlow::Node {
/** Gets a string that describes the type of this remote flow source. */
/** Gets a human-readable string that describes the type of this remote flow source. */
cached
abstract string getSourceType();
@@ -21,6 +21,57 @@ abstract class RemoteFlowSource extends DataFlow::Node {
predicate isUserControlledObject() { none() }
}
/**
* A type of remote flow source that is specific to the browser environment.
*/
class ClientSideRemoteFlowKind extends string {
ClientSideRemoteFlowKind() { this = ["query", "fragment", "path", "url", "name"] }
/**
* Holds if this is the `query` kind, describing sources derived from the query parameters of the browser URL,
* such as `location.search`.
*/
predicate isQuery() { this = "query" }
/**
* Holds if this is the `frgament` kind, describing sources derived from the fragment part of the browser URL,
* such as `location.hash`.
*/
predicate isFragment() { this = "fragment" }
/**
* Holds if this is the `path` kind, describing sources derived from the pathname of the browser URL,
* such as `location.pathname`.
*/
predicate isPath() { this = "path" }
/**
* Holds if this is the `url` kind, describing sources derived from the browser URL,
* where the untrusted part of the URL is prefixed by trusted data, such as the scheme and hostname.
*/
predicate isUrl() { this = "url" }
/** Holds if this is the `query` or `fragment` kind. */
predicate isQueryOrFragment() { this.isQuery() or this.isFragment() }
/** Holds if this is the `path`, `query`, or `fragment` kind. */
predicate isPathOrQueryOrFragment() { this.isPath() or this.isQuery() or this.isFragment() }
/** Holds if this is the `path` or `url` kind. */
predicate isPathOrUrl() { this.isPath() or this.isUrl() }
/** Holds if this is the `name` kind, describing sources derived from the window name, such as `window.name`. */
predicate isWindowName() { this = "name" }
}
/**
* A source of remote input in a web browser environment.
*/
abstract class ClientSideRemoteFlowSource extends RemoteFlowSource {
/** Gets a string indicating what part of the browser environment this was derived from. */
abstract ClientSideRemoteFlowKind getKind();
}
/**
* A specification of a remote flow source in a JSON file included in the database.
*

8
javascript/ql/src/semmle/javascript/security/dataflow/RequestForgeryCustomizations.qll
View File

@@ -33,7 +33,13 @@ module RequestForgery {
/** A source of remote user input, considered as a flow source for request forgery. */
private class RemoteFlowSourceAsSource extends Source {
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
RemoteFlowSourceAsSource() {
// Reduce FPs by excluding sources from client-side path or URL
exists(RemoteFlowSource src |
this = src and
not src.(ClientSideRemoteFlowSource).getKind().isPathOrUrl()
)
}
}
/**

7
javascript/ql/src/semmle/javascript/security/dataflow/TaintedPathCustomizations.qll
View File

@@ -557,7 +557,12 @@ module TaintedPath {
* tainted-path vulnerabilities.
*/
class RemoteFlowSourceAsSource extends Source {
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
RemoteFlowSourceAsSource() {
exists(RemoteFlowSource src |
this = src and
not src instanceof ClientSideRemoteFlowSource
)
}
}
/**

7
javascript/ql/src/semmle/javascript/security/dataflow/UnsafeDynamicMethodAccessCustomizations.qll
View File

@@ -58,13 +58,6 @@ module UnsafeDynamicMethodAccess {
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
}
/**
* The page URL considered as a flow source for unsafe dynamic method access.
*/
class DocumentUrlAsSource extends Source {
DocumentUrlAsSource() { this = DOM::locationSource() }
}
/**
* A function invocation of an unsafe function, as a sink for remote unsafe dynamic method access.
*/

20
javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll
View File

@@ -383,25 +383,7 @@ module DomBasedXss {
*/
class SafePropertyReadSanitizer extends Sanitizer, DataFlow::Node {
SafePropertyReadSanitizer() {
exists(PropAccess pacc | pacc = this.asExpr() |
isSafeLocationProperty(pacc)
or
pacc.getPropertyName() = "length"
)
}
}
/**
* A sanitizer that reads the first part a location split by "?", e.g. `location.href.split('?')[0]`.
*/
class QueryPrefixSanitizer extends Sanitizer {
StringSplitCall splitCall;
QueryPrefixSanitizer() {
this = splitCall.getASubstringRead(0) and
splitCall.getSeparator() = "?" and
splitCall.getBaseString().getALocalSource() =
[DOM::locationRef(), DOM::locationRef().getAPropertyRead("href")]
exists(PropAccess pacc | pacc = this.asExpr() | pacc.getPropertyName() = "length")
}
}