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

JS: Add TaintedUrlSuffixCustomizations

Browse Source 
Importing TaintedUrlSuffix.qll causes the flow label to materialised in unrelated queries, so:

- Renames TaintedUrlSuffix.qll to TaintedUrlSuffixCustomizations.qll
- Make the flow label class abstract
- Adds a new TaintedUrlSuffix.qll that re-exports the above file and also materialises the flow label
- Import the *Customizations.qll file from contexts where we don't want to materialise the flow label
This commit is contained in:
Asger F
2024-12-09 10:06:42 +01:00
parent d1694013ff
commit 2a2a4d2b67
4 changed files with 121 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffix.qll
View File

@@ -4,114 +4,15 @@
*/
import javascript
private import semmle.javascript.dataflow.internal.DataFlowPrivate as DataFlowPrivate
/**
* Provides a flow label for reasoning about URLs with a tainted query and fragment part,
* which we collectively refer to as the "suffix" of the URL.
*/
module TaintedUrlSuffix {
private import DataFlow
import TaintedUrlSuffixCustomizations::TaintedUrlSuffix
/**
* The flow label representing a URL with a tainted query and fragment part.
*
* Can also be accessed using `TaintedUrlSuffix::label()`.
*/
class TaintedUrlSuffixLabel extends FlowLabel {
TaintedUrlSuffixLabel() { this = "tainted-url-suffix" }
}
/**
* Gets the flow label representing a URL with a tainted query and fragment part.
*/
FlowLabel label() { result instanceof TaintedUrlSuffixLabel }
/** Gets a remote flow source that is a tainted URL query or fragment part from `window.location`. */
ClientSideRemoteFlowSource source() {
result = DOM::locationRef().getAPropertyRead(["search", "hash"])
or
result = DOM::locationSource()
or
result.getKind().isUrl()
}
/**
* Holds if `node` should be a barrier for the given `label`.
*
* This should be used in the `isBarrier` predicate of a configuration that uses the tainted-url-suffix
* label.
*/
predicate isBarrier(Node node, FlowLabel label) {
label = label() and
DataFlowPrivate::optionalBarrier(node, "split-url-suffix")
}
/**
* Holds if there is a flow step `src -> dst` involving the URL suffix taint label.
*
* This handles steps through string operations, promises, URL parsers, and URL accessors.
*/
predicate step(Node src, Node dst, FlowLabel srclbl, FlowLabel dstlbl) {
// Transition from tainted-url-suffix to general taint when entering the second array element
// of a split('#') or split('?') array.
//
// x [tainted-url-suffix] --> x.split('#') [array element 1] [taint]
//
// Technically we should also preverse tainted-url-suffix when entering the first array element of such
// a split, but this mostly leads to FPs since we currently don't track if the taint has been through URI-decoding.
// (The query/fragment parts are often URI-decoded in practice, but not the other URL parts are not)
srclbl = label() and
dstlbl.isTaint() and
DataFlowPrivate::optionalStep(src, "split-url-suffix-post", dst)
or
// Transition from URL suffix to full taint when extracting the query/fragment part.
srclbl = label() and
dstlbl.isTaint() and
(
exists(MethodCallNode call, string name |
src = call.getReceiver() and
dst = call and
name = call.getMethodName()
|
// Substring that is not a prefix
name = StringOps::substringMethodName() and
not call.getArgument(0).getIntValue() = 0
or
// Replace '#' and '?' with nothing
name = "replace" and
call.getArgument(0).getStringValue() = ["#", "?"] and
call.getArgument(1).getStringValue() = ""
or
// The `get` call in `url.searchParams.get(x)` and `url.hashParams.get(x)`
// The step should be safe since nothing else reachable by this flow label supports a method named 'get'.
name = "get"
or
// Methods on URL objects from the Closure library
name = "getDecodedQuery"
or
name = "getFragment"
or
name = "getParameterValue"
or
name = "getParameterValues"
or
name = "getQueryData"
)
or
exists(PropRead read |
src = read.getBase() and
dst = read and
// Unlike the `search` property, the `query` property from `url.parse` does not include the `?`.
read.getPropertyName() = "query"
)
or
// Assume calls to regexp.exec always extract query/fragment parameters.
exists(MethodCallNode call |
call = any(DataFlow::RegExpCreationNode re).getAMethodCall("exec") and
src = call.getArgument(0) and
dst = call
)
)
private class ConcreteTaintedUrlSuffixLabel extends TaintedUrlSuffixLabel {
ConcreteTaintedUrlSuffixLabel() { this = this }
}
}

117
javascript/ql/lib/semmle/javascript/security/TaintedUrlSuffixCustomizations.qll Normal file
View File

@@ -0,0 +1,117 @@
/**
* Provides a flow label for reasoning about URLs with a tainted query and fragment part,
* which we collectively refer to as the "suffix" of the URL.
*/
import javascript
private import semmle.javascript.dataflow.internal.DataFlowPrivate as DataFlowPrivate
/**
* Provides a flow label for reasoning about URLs with a tainted query and fragment part,
* which we collectively refer to as the "suffix" of the URL.
*/
module TaintedUrlSuffix {
private import DataFlow
/**
* The flow label representing a URL with a tainted query and fragment part.
*
* Can also be accessed using `TaintedUrlSuffix::label()`.
*/
abstract class TaintedUrlSuffixLabel extends FlowLabel {
TaintedUrlSuffixLabel() { this = "tainted-url-suffix" }
}
/**
* Gets the flow label representing a URL with a tainted query and fragment part.
*/
FlowLabel label() { result instanceof TaintedUrlSuffixLabel }
/** Gets a remote flow source that is a tainted URL query or fragment part from `window.location`. */
ClientSideRemoteFlowSource source() {
result = DOM::locationRef().getAPropertyRead(["search", "hash"])
or
result = DOM::locationSource()
or
result.getKind().isUrl()
}
/**
* Holds if `node` should be a barrier for the given `label`.
*
* This should be used in the `isBarrier` predicate of a configuration that uses the tainted-url-suffix
* label.
*/
predicate isBarrier(Node node, FlowLabel label) {
label = label() and
DataFlowPrivate::optionalBarrier(node, "split-url-suffix")
}
/**
* Holds if there is a flow step `src -> dst` involving the URL suffix taint label.
*
* This handles steps through string operations, promises, URL parsers, and URL accessors.
*/
predicate step(Node src, Node dst, FlowLabel srclbl, FlowLabel dstlbl) {
// Transition from tainted-url-suffix to general taint when entering the second array element
// of a split('#') or split('?') array.
//
// x [tainted-url-suffix] --> x.split('#') [array element 1] [taint]
//
// Technically we should also preverse tainted-url-suffix when entering the first array element of such
// a split, but this mostly leads to FPs since we currently don't track if the taint has been through URI-decoding.
// (The query/fragment parts are often URI-decoded in practice, but not the other URL parts are not)
srclbl = label() and
dstlbl.isTaint() and
DataFlowPrivate::optionalStep(src, "split-url-suffix-post", dst)
or
// Transition from URL suffix to full taint when extracting the query/fragment part.
srclbl = label() and
dstlbl.isTaint() and
(
exists(MethodCallNode call, string name |
src = call.getReceiver() and
dst = call and
name = call.getMethodName()
|
// Substring that is not a prefix
name = StringOps::substringMethodName() and
not call.getArgument(0).getIntValue() = 0
or
// Replace '#' and '?' with nothing
name = "replace" and
call.getArgument(0).getStringValue() = ["#", "?"] and
call.getArgument(1).getStringValue() = ""
or
// The `get` call in `url.searchParams.get(x)` and `url.hashParams.get(x)`
// The step should be safe since nothing else reachable by this flow label supports a method named 'get'.
name = "get"
or
// Methods on URL objects from the Closure library
name = "getDecodedQuery"
or
name = "getFragment"
or
name = "getParameterValue"
or
name = "getParameterValues"
or
name = "getQueryData"
)
or
exists(PropRead read |
src = read.getBase() and
dst = read and
// Unlike the `search` property, the `query` property from `url.parse` does not include the `?`.
read.getPropertyName() = "query"
)
or
// Assume calls to regexp.exec always extract query/fragment parameters.
exists(MethodCallNode call |
call = any(DataFlow::RegExpCreationNode re).getAMethodCall("exec") and
src = call.getArgument(0) and
dst = call
)
)
}
}

2
javascript/ql/lib/semmle/javascript/security/dataflow/ClientSideUrlRedirectCustomizations.qll
View File

@@ -5,7 +5,7 @@
*/
import javascript
private import semmle.javascript.security.TaintedUrlSuffix
private import semmle.javascript.security.TaintedUrlSuffixCustomizations
module ClientSideUrlRedirect {
/**

1
javascript/ql/test/library-tests/FlowLabels/DefaultFlowLabels.expected
View File

@@ -1,3 +1,2 @@
| data |
| taint |
| tainted-url-suffix |