Skip to content

Commit 89f4537

Browse files
committed
introduce StringSplitCall and use it
1 parent 9a7f8d9 commit 89f4537

File tree

4 files changed

+45
-17
lines changed

4 files changed

+45
-17
lines changed

javascript/ql/src/Security/CWE-020/IncompleteUrlSchemeCheck.ql

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,10 @@ class DangerousScheme extends string {
2727
/** Returns a node that refers to the scheme of `url`. */
2828
DataFlow::SourceNode schemeOf(DataFlow::Node url) {
2929
// url.split(":")[0]
30-
exists(DataFlow::MethodCallNode split |
31-
split.getMethodName() = "split" and
32-
split.getArgument(0).getStringValue() = ":" and
33-
result = split.getAPropertyRead("0") and
34-
url = split.getReceiver()
30+
exists(StringSplitCall split |
31+
split.getSplitAt() = ":" and
32+
result = split.getAnElementRead(0) and
33+
url = split.getUnsplit()
3534
)
3635
or
3736
// url.getScheme(), url.getProtocol(), getScheme(url), getProtocol(url)

javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,10 @@ import semmle.javascript.DynamicPropertyAccess
2121
*
2222
* We restrict this to parameter nodes to focus on "deep assignment" functions.
2323
*/
24-
class SplitCall extends MethodCallNode {
24+
class SplitCall extends StringSplitCall {
2525
SplitCall() {
26-
getMethodName() = "split" and
27-
getArgument(0).mayHaveStringValue(".") and
28-
getReceiver().getALocalSource() instanceof ParameterNode
26+
getSplitAt() = "." and
27+
getUnsplit() instanceof ParameterNode
2928
}
3029
}
3130

javascript/ql/src/semmle/javascript/StandardLibrary.qll

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,37 @@ class StringReplaceCall extends DataFlow::MethodCallNode {
146146
)
147147
}
148148
}
149+
150+
/**
151+
* A call to `String.prototype.split`.
152+
*
153+
* We heuristically include any call to a method called `split`, provided it either
154+
* has exactly one arguments, or local data flow suggests that the receiver may be a string.
155+
*/
156+
class StringSplitCall extends DataFlow::MethodCallNode {
157+
StringSplitCall() {
158+
this.getMethodName() = "split" and
159+
(getNumArgument() = 1 or getReceiver().mayHaveStringValue(_))
160+
}
161+
162+
/**
163+
* Gets a string that determines where the string is split.
164+
*/
165+
string getSplitAt() {
166+
getArgument(0).mayHaveStringValue(result)
167+
or
168+
result =
169+
getArgument(0).getALocalSource().(DataFlow::RegExpCreationNode).getRoot().getAMatchedString()
170+
}
171+
172+
/**
173+
* Gets a the SourceNode for the string before it is split.
174+
*/
175+
DataFlow::SourceNode getUnsplit() { result = getReceiver().getALocalSource() }
176+
177+
/**
178+
* Gets a read of the `i`th element from the split string.
179+
*/
180+
bindingset[i]
181+
DataFlow::Node getAnElementRead(int i) { result = getAPropertyRead(i.toString()) }
182+
}

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,16 +70,12 @@ module ClientSideUrlRedirect {
7070
* A sanitizer that reads the first part a location split by "?", e.g. `location.href.split('?')[0]`.
7171
*/
7272
class QueryPrefixSanitizer extends Sanitizer {
73-
DataFlow::PropRead read;
73+
StringSplitCall splitCall;
7474

7575
QueryPrefixSanitizer() {
76-
this = read and
77-
read.getPropertyName() = "0" and
78-
exists(DataFlow::MethodCallNode splitCall | splitCall = read.getBase().getALocalSource() |
79-
splitCall.getMethodName() = "split" and
80-
splitCall.getArgument(0).mayHaveStringValue("?") and
81-
splitCall.getReceiver() = [DOM::locationRef(), DOM::locationRef().getAPropertyRead("href")]
82-
)
76+
this = splitCall.getAnElementRead(0) and
77+
splitCall.getSplitAt() = "?" and
78+
splitCall.getUnsplit() = [DOM::locationRef(), DOM::locationRef().getAPropertyRead("href")]
8379
}
8480
}
8581

0 commit comments

Comments
 (0)