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

Merge branch 'master' into python-incomplete-url-sanitize

Browse Source
This commit is contained in:
Taus
2019-01-29 14:17:37 +01:00
committed by GitHub
parent 3850f87879 d776d9f903
commit 9adb19f3a9
283 changed files with 5075 additions and 2014 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
javascript/ql/src/React/InconsistentStateUpdate.qhelp
View File

@@ -36,7 +36,7 @@ Instead, the callback form of <code>setState</code> should be used:
</p>
<sample language="javascript">
this.setState((prevState) => ({
this.setState(prevState => ({
counter: prevState.counter + 1
}));
</sample>

4
javascript/ql/src/Security/CWE-116/DoubleEscaping.qhelp
View File

@@ -52,13 +52,13 @@ The following example shows a pair of hand-written HTML encoding and decoding fu
<p>
The encoding function correctly handles ampersand before the other characters. For example,
the string <code>me &amp; "you"</code> is encoded as <code>me &amp;amp; &amp;quot;you&amp;quot;</code>,
and the string <code>&quot;</code> is encoded as <code>&amp;quot;</code>.
and the string <code>&amp;quot;</code> is encoded as <code>&amp;amp;quot;</code>.
</p>
<p>
The decoding function, however, incorrectly decodes <code>&amp;amp;</code> into <code>&amp;</code>
before handling the other characters. So while it correctly decodes the first example above,
it decodes the second example (<code>&amp;quot;</code>) to <code>&quot;</code> (a single double quote),
it decodes the second example (<code>&amp;amp;quot;</code>) to <code>&quot;</code> (a single double quote),
which is not correct.
</p>

23
javascript/ql/src/Statements/UselessConditional.ql
View File

@@ -65,17 +65,24 @@ predicate isInitialParameterUse(Expr e) {
}
/**
* Holds if `e` directly uses the returned value from a function call that returns a constant boolean value.
* Holds if `e` directly uses the returned value from functions that return constant boolean values.
*/
predicate isConstantBooleanReturnValue(Expr e) {
// unlike `SourceNode.flowsTo` this will not include uses we have refinement information for
exists(DataFlow::CallNode call | exists(call.analyze().getTheBooleanValue()) |
e = call.asExpr()
or
// also support return values that are assigned to variables
exists(SsaExplicitDefinition ssa |
ssa.getDef().getSource() = call.asExpr() and
ssa.getVariable().getAUse() = e
exists(string b | (b = "true" or b = "false") |
forex(DataFlow::CallNode call, Expr ret |
ret = call.getACallee().getAReturnedExpr() and
(
e = call.asExpr()
or
// also support return values that are assigned to variables
exists(SsaExplicitDefinition ssa |
ssa.getDef().getSource() = call.asExpr() and
ssa.getVariable().getAUse() = e
)
)
|
ret.(BooleanLiteral).getValue() = b
)
)
or

2
javascript/ql/src/external/DuplicateToplevel.ql vendored
View File

@@ -22,5 +22,5 @@ from TopLevel one, TopLevel another, float percent
where
duplicateContainers(one, another, percent) and
one.getNumLines() > 5
select one.(FirstLineOf), percent + "% of statements in this script are duplicated in $@.",
select one.(FirstLineOf), percent.floor() + "% of statements in this script are duplicated in $@.",
another.(FirstLineOf), "another script"

1
javascript/ql/src/javascript.qll
View File

@@ -41,6 +41,7 @@ import semmle.javascript.SSA
import semmle.javascript.StandardLibrary
import semmle.javascript.Stmt
import semmle.javascript.StringConcatenation
import semmle.javascript.StringOps
import semmle.javascript.Templates
import semmle.javascript.Tokens
import semmle.javascript.TypeScript

21
javascript/ql/src/semmle/javascript/DefensiveProgramming.qll
View File

@@ -53,7 +53,7 @@ module Internal {
* `polarity` is true iff the inner expression is nested in an even number of negations.
*/
private Expr stripNotsAndParens(Expr e, boolean polarity) {
exists(Expr inner | inner = e.getUnderlyingValue() |
exists(Expr inner | inner = e.stripParens() |
if inner instanceof LogNotExpr
then result = stripNotsAndParens(inner.(LogNotExpr).getOperand(), polarity.booleanNot())
else (
@@ -199,11 +199,14 @@ module Internal {
Expr target;
UndefinedNullCrashUse() {
this.(InvokeExpr).getCallee().getUnderlyingValue() = target
or
this.(PropAccess).getBase().getUnderlyingValue() = target
or
this.(MethodCallExpr).getReceiver().getUnderlyingValue() = target
exists (Expr thrower |
stripNotsAndParens(this, _) = thrower |
thrower.(InvokeExpr).getCallee().getUnderlyingValue() = target
or
thrower.(PropAccess).getBase().getUnderlyingValue() = target
or
thrower.(MethodCallExpr).getReceiver().getUnderlyingValue() = target
)
}
/**
@@ -220,7 +223,8 @@ module Internal {
private class NonFunctionCallCrashUse extends Expr {
Expr target;
NonFunctionCallCrashUse() { this.(InvokeExpr).getCallee().getUnderlyingValue() = target }
NonFunctionCallCrashUse() {
stripNotsAndParens(this, _).(InvokeExpr).getCallee().getUnderlyingValue() = target }
/**
* Gets the subexpression that will cause an exception to be thrown if it is not a `function`.
@@ -273,7 +277,6 @@ module Internal {
guardVar.getVariable() = useVar.getVariable()
|
getAGuardedExpr(this.asExpr())
.getUnderlyingValue()
.(UndefinedNullCrashUse)
.getVulnerableSubexpression() = useVar and
// exclude types whose truthiness depend on the value
@@ -306,7 +309,6 @@ module Internal {
guardVar.getVariable() = useVar.getVariable()
|
getAGuardedExpr(guard)
.getUnderlyingValue()
.(UndefinedNullCrashUse)
.getVulnerableSubexpression() = useVar
)
@@ -375,7 +377,6 @@ module Internal {
guardVar.getVariable() = useVar.getVariable()
|
getAGuardedExpr(guard)
.getUnderlyingValue()
.(NonFunctionCallCrashUse)
.getVulnerableSubexpression() = useVar
) and

397
javascript/ql/src/semmle/javascript/StringOps.qll Normal file
View File

@@ -0,0 +1,397 @@
/**
* Provides classes and predicates for reasoning about string-manipulating expressions.
*/
import javascript
module StringOps {
/**
* A expression that is equivalent to `A.startsWith(B)` or `!A.startsWith(B)`.
*/
abstract class StartsWith extends DataFlow::Node {
/**
* Gets the `A` in `A.startsWith(B)`.
*/
abstract DataFlow::Node getBaseString();
/**
* Gets the `B` in `A.startsWith(B)`.
*/
abstract DataFlow::Node getSubstring();
/**
* Gets the polarity of the check.
*
* If the polarity is `false` the check returns `true` if the string does not start
* with the given substring.
*/
boolean getPolarity() { result = true }
}
/**
* An expression of form `A.startsWith(B)`.
*/
private class StartsWith_Native extends StartsWith, DataFlow::MethodCallNode {
StartsWith_Native() {
getMethodName() = "startsWith" and
getNumArgument() = 1
}
override DataFlow::Node getBaseString() {
result = getReceiver()
}
override DataFlow::Node getSubstring() {
result = getArgument(0)
}
}
/**
* An expression of form `A.indexOf(B) === 0`.
*/
private class StartsWith_IndexOfEquals extends StartsWith, DataFlow::ValueNode {
override EqualityTest astNode;
DataFlow::MethodCallNode indexOf;
StartsWith_IndexOfEquals() {
indexOf.getMethodName() = "indexOf" and
indexOf.getNumArgument() = 1 and
indexOf.flowsToExpr(astNode.getAnOperand()) and
astNode.getAnOperand().getIntValue() = 0
}
override DataFlow::Node getBaseString() {
result = indexOf.getReceiver()
}
override DataFlow::Node getSubstring() {
result = indexOf.getArgument(0)
}
override boolean getPolarity() {
result = astNode.getPolarity()
}
}
/**
* An expression of form `A.indexOf(B)` coerced to a boolean.
*
* This is equivalent to `!A.startsWith(B)` since all return values other than zero map to `true`.
*/
private class StartsWith_IndexOfCoercion extends StartsWith, DataFlow::MethodCallNode {
StartsWith_IndexOfCoercion() {
getMethodName() = "indexOf" and
getNumArgument() = 1 and
this.flowsToExpr(any(ConditionGuardNode guard).getTest()) // check for boolean coercion
}
override DataFlow::Node getBaseString() {
result = getReceiver()
}
override DataFlow::Node getSubstring() {
result = getArgument(0)
}
override boolean getPolarity() {
result = false
}
}
/**
* A call of form `_.startsWith(A, B)` or `ramda.startsWith(A, B)`.
*/
private class StartsWith_Library extends StartsWith, DataFlow::CallNode {
StartsWith_Library() {
getNumArgument() = 2 and
exists (DataFlow::SourceNode callee | this = callee.getACall() |
callee = LodashUnderscore::member("startsWith") or
callee = DataFlow::moduleMember("ramda", "startsWith")
)
}
override DataFlow::Node getBaseString() {
result = getArgument(0)
}
override DataFlow::Node getSubstring() {
result = getArgument(1)
}
}
/**
* A comparison of form `x[0] === "k"` for some single-character constant `k`.
*/
private class StartsWith_FirstCharacter extends StartsWith, DataFlow::ValueNode {
override EqualityTest astNode;
DataFlow::PropRead read;
Expr constant;
StartsWith_FirstCharacter() {
read.flowsTo(astNode.getAnOperand().flow()) and
read.getPropertyNameExpr().getIntValue() = 0 and
constant.getStringValue().length() = 1 and
astNode.getAnOperand() = constant
}
override DataFlow::Node getBaseString() {
result = read.getBase()
}
override DataFlow::Node getSubstring() {
result = constant.flow()
}
override boolean getPolarity() {
result = astNode.getPolarity()
}
}
/**
* A comparison of form `x.substring(0, y.length) === y`.
*/
private class StartsWith_Substring extends StartsWith, DataFlow::ValueNode {
override EqualityTest astNode;
DataFlow::MethodCallNode call;
DataFlow::Node substring;
StartsWith_Substring() {
astNode.hasOperands(call.asExpr(), substring.asExpr()) and
(call.getMethodName() = "substring" or call.getMethodName() = "substr") and
call.getNumArgument() = 2 and
(
substring.getALocalSource().getAPropertyRead("length").flowsTo(call.getArgument(1))
or
substring.asExpr().getStringValue().length() = call.getArgument(1).asExpr().getIntValue()
)
}
override DataFlow::Node getBaseString() {
result = call.getReceiver()
}
override DataFlow::Node getSubstring() {
result = substring
}
override boolean getPolarity() {
result = astNode.getPolarity()
}
}
/**
* A expression that is equivalent to `A.includes(B)` or `!A.includes(B)`.
*
* Note that this also includes calls to the array method named `includes`.
*/
abstract class Includes extends DataFlow::Node {
/** Gets the `A` in `A.includes(B)`. */
abstract DataFlow::Node getBaseString();
/** Gets the `B` in `A.includes(B)`. */
abstract DataFlow::Node getSubstring();
/**
* Gets the polarity of the check.
*
* If the polarity is `false` the check returns `true` if the string does not contain
* the given substring.
*/
boolean getPolarity() { result = true }
}
/**
* A call to a method named `includes`, assumed to refer to `String.prototype.includes`.
*/
private class Includes_Native extends Includes, DataFlow::MethodCallNode {
Includes_Native() {
getMethodName() = "includes" and
getNumArgument() = 1
}
override DataFlow::Node getBaseString() {
result = getReceiver()
}
override DataFlow::Node getSubstring() {
result = getArgument(0)
}
}
/**
* A call to `_.includes` or similar, assumed to operate on strings.
*/
private class Includes_Library extends Includes, DataFlow::CallNode {
Includes_Library() {
exists (string name |
this = LodashUnderscore::member(name).getACall() and
(name = "includes" or name = "include" or name = "contains")
)
}
override DataFlow::Node getBaseString() {
result = getArgument(0)
}
override DataFlow::Node getSubstring() {
result = getArgument(1)
}
}
/**
* A check of form `A.indexOf(B) !== -1` or similar.
*/
private class Includes_IndexOfEquals extends Includes, DataFlow::ValueNode {
MethodCallExpr indexOf;
override EqualityTest astNode;
Includes_IndexOfEquals() {
exists (Expr index | astNode.hasOperands(indexOf, index) |
// one operand is of the form `whitelist.indexOf(x)`
indexOf.getMethodName() = "indexOf" and
// and the other one is -1
index.getIntValue() = -1
)
}
override DataFlow::Node getBaseString() {
result = indexOf.getReceiver().flow()
}
override DataFlow::Node getSubstring() {
result = indexOf.getArgument(0).flow()
}
override boolean getPolarity() {
result = astNode.getPolarity().booleanNot()
}
}
/**
* A check of form `A.indexOf(B) >= 0` or similar.
*/
private class Includes_IndexOfRelational extends Includes, DataFlow::ValueNode {
MethodCallExpr indexOf;
override RelationalComparison astNode;
boolean polarity;
Includes_IndexOfRelational() {
exists (Expr lesser, Expr greater |
astNode.getLesserOperand() = lesser and
astNode.getGreaterOperand() = greater and
indexOf.getMethodName() = "indexOf" and
indexOf.getNumArgument() = 1 |
polarity = true and
greater = indexOf and
(
lesser.getIntValue() = 0 and astNode.isInclusive()
or
lesser.getIntValue() = -1 and not astNode.isInclusive()
)
or
polarity = false and
lesser = indexOf and
(
greater.getIntValue() = -1 and astNode.isInclusive()
or
greater.getIntValue() = 0 and not astNode.isInclusive()
)
)
}
override DataFlow::Node getBaseString() {
result = indexOf.getReceiver().flow()
}
override DataFlow::Node getSubstring() {
result = indexOf.getArgument(0).flow()
}
override boolean getPolarity() {
result = polarity
}
}
/**
* An expression of form `~A.indexOf(B)` which, when coerced to a boolean, is equivalent to `A.includes(B)`.
*/
private class Includes_IndexOfBitwise extends Includes, DataFlow::ValueNode {
MethodCallExpr indexOf;
override BitNotExpr astNode;
Includes_IndexOfBitwise() {
astNode.getOperand() = indexOf and
indexOf.getMethodName() = "indexOf"
}
override DataFlow::Node getBaseString() {
result = indexOf.getReceiver().flow()
}
override DataFlow::Node getSubstring() {
result = indexOf.getArgument(0).flow()
}
}
/**
* An expression that is equivalent to `A.endsWith(B)` or `!A.endsWith(B)`.
*/
abstract class EndsWith extends DataFlow::Node {
/**
* Gets the `A` in `A.startsWith(B)`.
*/
abstract DataFlow::Node getBaseString();
/**
* Gets the `B` in `A.startsWith(B)`.
*/
abstract DataFlow::Node getSubstring();
/**
* Gets the polarity if the check.
*
* If the polarity is `false` the check returns `true` if the string does not end
* with the given substring.
*/
boolean getPolarity() { result = true }
}
/**
* A call of form `A.endsWith(B)`.
*/
private class EndsWith_Native extends EndsWith, DataFlow::MethodCallNode {
EndsWith_Native() {
getMethodName() = "endsWith" and
getNumArgument() = 1
}
override DataFlow::Node getBaseString() {
result = getReceiver()
}
override DataFlow::Node getSubstring() {
result = getArgument(0)
}
}
/**
* A call of form `_.endsWith(A, B)` or `ramda.endsWith(A, B)`.
*/
private class EndsWith_Library extends StartsWith, DataFlow::CallNode {
EndsWith_Library() {
getNumArgument() = 2 and
exists (DataFlow::SourceNode callee | this = callee.getACall() |
callee = LodashUnderscore::member("endsWith") or
callee = DataFlow::moduleMember("ramda", "endsWith")
)
}
override DataFlow::Node getBaseString() {
result = getArgument(0)
}
override DataFlow::Node getSubstring() {
result = getArgument(1)
}
}
}

74
javascript/ql/src/semmle/javascript/dataflow/Configuration.qll
View File

@@ -660,28 +660,82 @@ private predicate flowThroughProperty(
* All of this is done under configuration `cfg`, and `arg` flows along a path
* summarized by `summary`, while `cb` is only tracked locally.
*/
private predicate higherOrderCall(
private predicate summarizedHigherOrderCall(
DataFlow::Node arg, DataFlow::Node cb, int i, DataFlow::Configuration cfg, PathSummary summary
) {
exists (Function f, DataFlow::InvokeNode outer, DataFlow::InvokeNode inner, int j,
DataFlow::Node innerArg, DataFlow::ParameterNode cbParm, PathSummary oldSummary |
exists(
Function f, DataFlow::InvokeNode outer, DataFlow::InvokeNode inner, int j,
DataFlow::Node innerArg, DataFlow::ParameterNode cbParm, PathSummary oldSummary
|
reachableFromInput(f, outer, arg, innerArg, cfg, oldSummary) and
argumentPassing(outer, cb, f, cbParm) and
innerArg = inner.getArgument(j) |
innerArg = inner.getArgument(j)
|
// direct higher-order call
cbParm.flowsTo(inner.getCalleeNode()) and
i = j and
summary = oldSummary
or
// indirect higher-order call
exists (DataFlow::Node cbArg, PathSummary newSummary |
exists(DataFlow::Node cbArg, PathSummary newSummary |
cbParm.flowsTo(cbArg) and
higherOrderCall(innerArg, cbArg, i, cfg, newSummary) and
summarizedHigherOrderCall(innerArg, cbArg, i, cfg, newSummary) and
summary = oldSummary.append(PathSummary::call()).append(newSummary)
)
)
}
/**
* Holds if `arg` is passed as the `i`th argument to `callback` through a callback invocation.
*
* This can be a summarized call, that is, `arg` and `callback` flow into a call,
* `f(arg, callback)`, which performs the invocation.
*
* Alternatively, the callback can flow into a call `f(callback)` which itself provides the `arg`.
* That is, `arg` refers to a value defined in `f` or one of its callees.
*
* In the latter case, the summary will consists of both a `return` and `call` step, for the following reasons:
*
* - Having `return` in the summary ensures that arguments passsed to `f` can't propagate back out along this edge.
* This is, `arg` should be defined in `f` or one of its callees, since a context-dependent value (i.e. parameter)
* should not propagate to every callback passed to `f`.
* In reality, `arg` may refer to a parameter, but in that case, the `return` summary prevents the edge from ever
* being used.
*
* - Having `call` in the summary ensures that values we propagate into the callback definition along this edge
* can't propagate out to other callers of that function through a return statement.
*
* - The flow label mapping of the summary corresponds to the transformation from `arg` to the
* invocation of the callback.
*/
predicate higherOrderCall(
DataFlow::Node arg, DataFlow::SourceNode callback, int i, DataFlow::Configuration cfg,
PathSummary summary
) {
// Summarized call
exists(DataFlow::Node cb |
summarizedHigherOrderCall(arg, cb, i, cfg, summary) and
callback.flowsTo(cb)
)
or
// Local invocation of a parameter
isRelevant(arg, cfg) and
exists(DataFlow::InvokeNode invoke |
arg = invoke.getArgument(i) and
invoke = callback.(DataFlow::ParameterNode).getACall() and
summary = PathSummary::call()
)
or
// Forwarding of the callback parameter (but not the argument).
exists(DataFlow::Node cbArg, DataFlow::SourceNode innerCb, PathSummary oldSummary |
higherOrderCall(arg, innerCb, i, cfg, oldSummary) and
callStep(cbArg, innerCb) and
callback.flowsTo(cbArg) and
// Prepend a 'return' summary to prevent context-dependent values (i.e. parameters) from using this edge.
summary = PathSummary::return().append(oldSummary)
)
}
/**
* Holds if `pred` is passed as an argument to a function `f` which also takes a
* callback parameter `cb` and then invokes `cb`, passing `pred` into parameter `succ`
@@ -693,12 +747,8 @@ private predicate higherOrderCall(
private predicate flowIntoHigherOrderCall(
DataFlow::Node pred, DataFlow::Node succ, DataFlow::Configuration cfg, PathSummary summary
) {
exists(
DataFlow::Node fArg, DataFlow::FunctionNode cb,
int i, PathSummary oldSummary
|
higherOrderCall(pred, fArg, i, cfg, oldSummary) and
cb = fArg.getALocalSource() and
exists(DataFlow::FunctionNode cb, int i, PathSummary oldSummary |
higherOrderCall(pred, cb, i, cfg, oldSummary) and
succ = cb.getParameter(i) and
summary = oldSummary.append(PathSummary::call())
)

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

@@ -593,7 +593,9 @@ module TaintTracking {
/**
* A check of the form `if(o.<contains>(x))`, which sanitizes `x` in its "then" branch.
*
* `<contains>` is one of: `contains`, `has`, `hasOwnProperty`, `includes`
* `<contains>` is one of: `contains`, `has`, `hasOwnProperty`
*
* Note that the `includes` method is covered by `StringInclusionSanitizer`.
*/
class WhitelistContainmentCallSanitizer extends AdditionalSanitizerGuardNode,
DataFlow::MethodCallNode {
@@ -601,8 +603,7 @@ module TaintTracking {
exists(string name |
name = "contains" or
name = "has" or
name = "hasOwnProperty" or
name = "includes"
name = "hasOwnProperty"
|
getMethodName() = name
)
@@ -673,86 +674,35 @@ module TaintTracking {
override predicate appliesTo(Configuration cfg) { any() }
}
/** A check of the form `if(whitelist.indexOf(x) != -1)`, which sanitizes `x` in its "then" branch. */
class IndexOfSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
MethodCallExpr indexOf;
/** A check of the form `whitelist.includes(x)` or equivalent, which sanitizes `x` in its "then" branch. */
class StringInclusionSanitizer extends AdditionalSanitizerGuardNode {
StringOps::Includes includes;
override EqualityTest astNode;
IndexOfSanitizer() {
exists(Expr index | astNode.hasOperands(indexOf, index) |
// one operand is of the form `whitelist.indexOf(x)`
indexOf.getMethodName() = "indexOf" and
// and the other one is -1
index.getIntValue() = -1
)
}
StringInclusionSanitizer() { this = includes }
override predicate sanitizes(boolean outcome, Expr e) {
outcome = astNode.getPolarity().booleanNot() and
e = indexOf.getArgument(0)
outcome = includes.getPolarity() and
e = includes.getSubstring().asExpr()
}
override predicate appliesTo(Configuration cfg) { any() }
}
/**
* A check of the form `if(whitelist.indexOf(x) >= 0)`, which sanitizes `x` in its "then" branch.
* A check of form `x.indexOf(y) > 0` or similar, which sanitizes `y` in the "then" branch.
*
* Similar relational checks are also supported.
* The more typical case of `x.indexOf(y) >= 0` is covered by `StringInclusionSanitizer`.
*/
private class RelationalIndexOfSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
class PositiveIndexOfSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
MethodCallExpr indexOf;
override RelationalComparison astNode;
boolean polarity;
RelationalIndexOfSanitizer() {
exists(Expr lesser, Expr greater |
astNode.getLesserOperand() = lesser and
astNode.getGreaterOperand() = greater and
indexOf.getMethodName() = "indexOf"
|
polarity = true and
greater = indexOf and
(
lesser.getIntValue() >= 0
or
lesser.getIntValue() = -1 and not astNode.isInclusive()
)
or
polarity = false and
lesser = indexOf and
(
greater.getIntValue() = -1
or
greater.getIntValue() = 0 and not astNode.isInclusive()
)
)
}
override predicate sanitizes(boolean outcome, Expr e) {
outcome = polarity and
e = indexOf.getArgument(0)
}
override predicate appliesTo(Configuration cfg) { any() }
}
/**
* A check of the form `if(~whitelist.indexOf(x))`, which sanitizes `x` in its "then" branch.
*
* This sanitizer is equivalent to `if(whitelist.indexOf(x) != -1)`, since `~n = 0` iff `n = -1`.
*/
private class BitwiseIndexOfSanitizer extends AdditionalSanitizerGuardNode, DataFlow::ValueNode {
MethodCallExpr indexOf;
override BitNotExpr astNode;
BitwiseIndexOfSanitizer() {
astNode.getOperand() = indexOf and
indexOf.getMethodName() = "indexOf"
PositiveIndexOfSanitizer() {
indexOf.getMethodName() = "indexOf" and
exists (int bound |
astNode.getGreaterOperand() = indexOf and
astNode.getLesserOperand().getIntValue() = bound and
bound >= 0)
}
override predicate sanitizes(boolean outcome, Expr e) {

61
javascript/ql/src/semmle/javascript/dataflow/TrackedNodes.qll
View File

@@ -216,28 +216,69 @@ private module NodeTracking {
* invokes `cb`, passing `arg` as its `i`th argument. `arg` flows along a path summarized
* by `summary`, while `cb` is only tracked locally.
*/
private predicate higherOrderCall(
private predicate summarizedHigherOrderCall(
DataFlow::Node arg, DataFlow::Node cb, int i, PathSummary summary
) {
exists (Function f, DataFlow::InvokeNode outer, DataFlow::InvokeNode inner, int j,
DataFlow::Node innerArg, DataFlow::ParameterNode cbParm, PathSummary oldSummary |
exists(
Function f, DataFlow::InvokeNode outer, DataFlow::InvokeNode inner, int j,
DataFlow::Node innerArg, DataFlow::ParameterNode cbParm, PathSummary oldSummary
|
reachableFromInput(f, outer, arg, innerArg, oldSummary) and
argumentPassing(outer, cb, f, cbParm) and
innerArg = inner.getArgument(j) |
innerArg = inner.getArgument(j)
|
// direct higher-order call
cbParm.flowsTo(inner.getCalleeNode()) and
i = j and
summary = oldSummary
or
// indirect higher-order call
exists (DataFlow::Node cbArg, PathSummary newSummary |
exists(DataFlow::Node cbArg, PathSummary newSummary |
cbParm.flowsTo(cbArg) and
higherOrderCall(innerArg, cbArg, i, newSummary) and
summarizedHigherOrderCall(innerArg, cbArg, i, newSummary) and
summary = oldSummary.append(PathSummary::call()).append(newSummary)
)
)
}
/**
* Holds if `arg` is passed as the `i`th argument to `callback` through a callback invocation.
*
* This can be a summarized call, that is, `arg` and `callback` flow into a call,
* `f(arg, callback)`, which performs the invocation.
*
* Alternatively, the callback can flow into a call `f(callback)` which itself provides the `arg`.
* That is, `arg` refers to a value defined in `f` or one of its callees.
*/
predicate higherOrderCall(
DataFlow::Node arg, DataFlow::SourceNode callback, int i, PathSummary summary
) {
// Summarized call
exists(DataFlow::Node cb |
summarizedHigherOrderCall(arg, cb, i, summary) and
callback.flowsTo(cb)
)
or
// Local invocation of a parameter
isRelevant(arg) and
exists(DataFlow::InvokeNode invoke |
arg = invoke.getArgument(i) and
invoke = callback.(DataFlow::ParameterNode).getACall() and
summary = PathSummary::call()
)
or
// Forwarding of the callback parameter (but not the argument).
// We use a return summary since flow moves back towards the call site.
// This ensures that an argument that is only tainted in some contexts cannot flow
// out to every callback.
exists(DataFlow::Node cbArg, DataFlow::SourceNode innerCb, PathSummary oldSummary |
higherOrderCall(arg, innerCb, i, oldSummary) and
callStep(cbArg, innerCb) and
callback.flowsTo(cbArg) and
summary = PathSummary::return().append(oldSummary)
)
}
/**
* Holds if `pred` is passed as an argument to a function `f` which also takes a
* callback parameter `cb` and then invokes `cb`, passing `pred` into parameter `succ`
@@ -247,12 +288,8 @@ private module NodeTracking {
private predicate flowIntoHigherOrderCall(
DataFlow::Node pred, DataFlow::Node succ, PathSummary summary
) {
exists(
DataFlow::Node fArg, DataFlow::FunctionNode cb,
int i, PathSummary oldSummary
|
higherOrderCall(pred, fArg, i, oldSummary) and
cb = fArg.getALocalSource() and
exists(DataFlow::FunctionNode cb, int i, PathSummary oldSummary |
higherOrderCall(pred, cb, i, oldSummary) and
succ = cb.getParameter(i) and
summary = oldSummary.append(PathSummary::call())
)

12
javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll
View File

@@ -109,9 +109,7 @@ predicate argumentPassing(
* Holds if there is a flow step from `pred` to `succ` through parameter passing
* to a function call.
*/
predicate callStep(DataFlow::Node pred, DataFlow::Node succ) {
argumentPassing(_, pred, _, succ)
}
predicate callStep(DataFlow::Node pred, DataFlow::Node succ) { argumentPassing(_, pred, _, succ) }
/**
* Holds if there is a flow step from `pred` to `succ` through returning
@@ -258,12 +256,18 @@ predicate loadStep(DataFlow::Node pred, DataFlow::PropRead succ, string prop) {
* invocation.
*/
predicate callback(DataFlow::Node arg, DataFlow::SourceNode cb) {
exists (DataFlow::InvokeNode invk, DataFlow::ParameterNode cbParm, DataFlow::Node cbArg |
exists(DataFlow::InvokeNode invk, DataFlow::ParameterNode cbParm, DataFlow::Node cbArg |
arg = invk.getAnArgument() and
cbParm.flowsTo(invk.getCalleeNode()) and
callStep(cbArg, cbParm) and
cb.flowsTo(cbArg)
)
or
exists(DataFlow::ParameterNode cbParm, DataFlow::Node cbArg |
callback(arg, cbParm) and
callStep(cbArg, cbParm) and
cb.flowsTo(cbArg)
)
}
/**

1
javascript/ql/test/library-tests/StringOps/EndsWith/EndsWith.expected Normal file
View File

@@ -0,0 +1 @@
| tst.js:5:7:5:19 | A.endsWith(B) | tst.js:5:7:5:7 | A | tst.js:5:18:5:18 | B | true |

4
javascript/ql/test/library-tests/StringOps/EndsWith/EndsWith.ql Normal file
View File

@@ -0,0 +1,4 @@
import javascript
from StringOps::EndsWith endsWith
select endsWith, endsWith.getBaseString(), endsWith.getSubstring(), endsWith.getPolarity()

8
javascript/ql/test/library-tests/StringOps/EndsWith/tst.js Normal file
View File

@@ -0,0 +1,8 @@
import * as _ from 'underscore';
import * as R from 'ramda';
function test() {
if (A.endsWith(B)) {}
if (_.endsWith(A, B)) {}
if (R.endsWith(A, B)) {}
}

7
javascript/ql/test/library-tests/StringOps/Includes/Includes.expected Normal file
View File

@@ -0,0 +1,7 @@
| tst.js:4:7:4:19 | A.includes(B) | tst.js:4:7:4:7 | A | tst.js:4:18:4:18 | B | true |
| tst.js:5:7:5:22 | _.includes(A, B) | tst.js:5:18:5:18 | A | tst.js:5:21:5:21 | B | true |
| tst.js:6:7:6:25 | A.indexOf(B) !== -1 | tst.js:6:7:6:7 | A | tst.js:6:17:6:17 | B | true |
| tst.js:7:7:7:23 | A.indexOf(B) >= 0 | tst.js:7:7:7:7 | A | tst.js:7:17:7:17 | B | true |
| tst.js:8:7:8:19 | ~A.indexOf(B) | tst.js:8:8:8:8 | A | tst.js:8:18:8:18 | B | true |
| tst.js:11:7:11:25 | A.indexOf(B) === -1 | tst.js:11:7:11:7 | A | tst.js:11:17:11:17 | B | false |
| tst.js:12:7:12:22 | A.indexOf(B) < 0 | tst.js:12:7:12:7 | A | tst.js:12:17:12:17 | B | false |

4
javascript/ql/test/library-tests/StringOps/Includes/Includes.ql Normal file
View File

@@ -0,0 +1,4 @@
import javascript
from StringOps::Includes include
select include, include.getBaseString(), include.getSubstring(), include.getPolarity()

18
javascript/ql/test/library-tests/StringOps/Includes/tst.js Normal file
View File

@@ -0,0 +1,18 @@
import * as _ from 'lodash';
function test() {
if (A.includes(B)) {}
if (_.includes(A, B)) {}
if (A.indexOf(B) !== -1) {}
if (A.indexOf(B) >= 0) {}
if (~A.indexOf(B)) {}
// negated
if (A.indexOf(B) === -1) {}
if (A.indexOf(B) < 0) {}
// non-examples
if (A.indexOf(B) === 0) {}
if (A.indexOf(B) !== 0) {}
if (A.indexOf(B) > 0) {}
}

14
javascript/ql/test/library-tests/StringOps/StartsWith/StartsWith.expected Normal file
View File

@@ -0,0 +1,14 @@
| tst.js:5:9:5:23 | A.startsWith(B) | tst.js:5:9:5:9 | A | tst.js:5:22:5:22 | B | true |
| tst.js:6:9:6:26 | _.startsWith(A, B) | tst.js:6:22:6:22 | A | tst.js:6:25:6:25 | B | true |
| tst.js:7:9:7:26 | R.startsWith(A, B) | tst.js:7:22:7:22 | A | tst.js:7:25:7:25 | B | true |
| tst.js:8:9:8:26 | A.indexOf(B) === 0 | tst.js:8:9:8:9 | A | tst.js:8:19:8:19 | B | true |
| tst.js:9:9:9:26 | A.indexOf(B) !== 0 | tst.js:9:9:9:9 | A | tst.js:9:19:9:19 | B | false |
| tst.js:10:9:10:26 | 0 !== A.indexOf(B) | tst.js:10:15:10:15 | A | tst.js:10:25:10:25 | B | false |
| tst.js:11:9:11:25 | 0 != A.indexOf(B) | tst.js:11:14:11:14 | A | tst.js:11:24:11:24 | B | false |
| tst.js:12:9:12:20 | A.indexOf(B) | tst.js:12:9:12:9 | A | tst.js:12:19:12:19 | B | false |
| tst.js:13:10:13:21 | A.indexOf(B) | tst.js:13:10:13:10 | A | tst.js:13:20:13:20 | B | false |
| tst.js:14:11:14:22 | A.indexOf(B) | tst.js:14:11:14:11 | A | tst.js:14:21:14:21 | B | false |
| tst.js:15:9:15:38 | A.subst ... ) === B | tst.js:15:9:15:9 | A | tst.js:15:38:15:38 | B | true |
| tst.js:16:9:16:38 | A.subst ... ) !== B | tst.js:16:9:16:9 | A | tst.js:16:38:16:38 | B | false |
| tst.js:17:9:17:35 | A.subst ... ) === B | tst.js:17:9:17:9 | A | tst.js:17:35:17:35 | B | true |
| tst.js:18:9:18:36 | A.subst ... "web/" | tst.js:18:9:18:9 | A | tst.js:18:31:18:36 | "web/" | true |

4
javascript/ql/test/library-tests/StringOps/StartsWith/StartsWith.ql Normal file
View File

@@ -0,0 +1,4 @@
import javascript
from StringOps::StartsWith check
select check, check.getBaseString(), check.getSubstring(), check.getPolarity()

30
javascript/ql/test/library-tests/StringOps/StartsWith/tst.js Normal file
View File

@@ -0,0 +1,30 @@
import * as _ from 'lodash';
import * as R from 'ramda';
function f(A, B) {
if (A.startsWith(B)) {}
if (_.startsWith(A, B)) {}
if (R.startsWith(A, B)) {}
if (A.indexOf(B) === 0) {}
if (A.indexOf(B) !== 0) {}
if (0 !== A.indexOf(B)) {}
if (0 != A.indexOf(B)) {}
if (A.indexOf(B)) {} // !startsWith
if (!A.indexOf(B)) {} // startsWith
if (!!A.indexOf(B)) {} // !startsWith
if (A.substring(0, B.length) === B) {}
if (A.substring(0, B.length) !== B) {}
if (A.substr(0, B.length) === B) {}
if (A.substring(0, 4) === "web/") {}
// non-examples
if (_.startsWith(A, B, 2)) {}
if (A.indexOf(B) >= 0) {}
if (A.indexOf(B) === 1) {}
if (A.indexOf(B) === A.indexOf(B)) {}
if (A.indexOf(B) !== -1) {}
if (A.indexOf(B, 2) === 0) {}
if (A.indexOf(B, 2)) {}
if (~A.indexOf(B)) {} // checks for existence, not startsWith
if (A.substring(B.length) === 0) {}
}

11
javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected
View File

@@ -1,10 +1,21 @@
| advanced-callgraph.js:2:13:2:20 | source() | advanced-callgraph.js:6:22:6:22 | v |
| callbacks.js:4:6:4:13 | source() | callbacks.js:34:27:34:27 | x |
| callbacks.js:4:6:4:13 | source() | callbacks.js:35:27:35:27 | x |
| callbacks.js:5:6:5:13 | source() | callbacks.js:34:27:34:27 | x |
| callbacks.js:5:6:5:13 | source() | callbacks.js:35:27:35:27 | x |
| callbacks.js:25:16:25:23 | source() | callbacks.js:47:26:47:26 | x |
| callbacks.js:25:16:25:23 | source() | callbacks.js:48:26:48:26 | x |
| callbacks.js:37:17:37:24 | source() | callbacks.js:37:37:37:37 | x |
| callbacks.js:44:17:44:24 | source() | callbacks.js:41:10:41:10 | x |
| callbacks.js:50:18:50:25 | source() | callbacks.js:30:29:30:29 | y |
| callbacks.js:51:18:51:25 | source() | callbacks.js:30:29:30:29 | y |
| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:18:8:18:14 | c.taint |
| constructor-calls.js:4:18:4:25 | source() | constructor-calls.js:22:8:22:19 | c_safe.taint |
| constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:26:8:26:14 | d.taint |
| constructor-calls.js:10:16:10:23 | source() | constructor-calls.js:30:8:30:19 | d_safe.taint |
| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:17:8:17:14 | c.param |
| constructor-calls.js:14:15:14:22 | source() | constructor-calls.js:25:8:25:14 | d.param |
| indexOf.js:4:11:4:18 | source() | indexOf.js:9:10:9:10 | x |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:17:14:17:14 | x |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:20:14:20:14 | y |
| partialCalls.js:4:17:4:24 | source() | partialCalls.js:30:14:30:20 | x.value |

52
javascript/ql/test/library-tests/TaintTracking/callbacks.js Normal file
View File

@@ -0,0 +1,52 @@
import * as dummy from 'dummy'; // treat as module
function provideTaint(cb) {
cb(source());
cb(source());
}
function provideTaint2(cb) {
provideTaint(cb);
provideTaint(cb); // suppress precision gains from functions with unique call site
}
function forwardTaint(x, cb) {
cb(x);
cb(x);
}
function forwardTaint2(x, cb) {
forwardTaint(x, cb);
forwardTaint(x, cb);
}
function middleSource(cb) {
// The source occurs in-between the callback definition and the callback invocation.
forwardTaint(source(), cb);
}
function middleCallback(x) {
// The callback definition occurs in-between the source and the callback invocation.
forwardTaint(x, y => sink(y)); // NOT OK
}
function test() {
provideTaint2(x => sink(x)); // NOT OK
provideTaint2(x => sink(x)); // NOT OK
forwardTaint2(source(), x => sink(x)); // NOT OK
forwardTaint2("safe", x => sink(x)); // OK
function helper1(x) {
sink(x); // NOT OK
return x;
}
forwardTaint2(source(), helper1);
sink(helper1("safe")); // OK
middleSource(x => sink(x)); // NOT OK
middleSource(x => sink(x)); // NOT OK
middleCallback(source());
middleCallback(source());
}

15
javascript/ql/test/library-tests/TaintTracking/indexOf.js Normal file
View File

@@ -0,0 +1,15 @@
let whitelist = ['a', 'b', 'c'];
function test() {
let x = source();
if (whitelist.indexOf(x) < -1) {
// unreachable
} else {
sink(x); // NOT OK
}
if (whitelist.indexOf(x) > 1) {
sink(x) // OK
}
}

2
javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UnneededDefensiveProgramming.expected
View File

@@ -70,3 +70,5 @@
| tst.js:140:13:140:22 | x === null | This guard always evaluates to false. |
| tst.js:156:23:156:31 | x != null | This guard always evaluates to true. |
| tst.js:158:13:158:21 | x != null | This guard always evaluates to true. |
| tst.js:177:2:177:2 | u | This guard always evaluates to false. |
| tst.js:178:2:178:2 | u | This guard always evaluates to false. |

2
javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/UselessConditional.expected
View File

@@ -0,0 +1,2 @@
| tst.js:175:2:175:2 | u | This use of variable 'u' always evaluates to false. |
| tst.js:176:2:176:2 | u | This use of variable 'u' always evaluates to false. |

5
javascript/ql/test/query-tests/Expressions/UnneededDefensiveProgramming/tst.js
View File

@@ -171,4 +171,9 @@
if (typeof window !== "undefined" && window.document);
if (typeof module !== "undefined" && module.exports);
if (typeof global !== "undefined" && global.process);
u && (f(), u.p);
u && (u.p, f()); // technically not OK, but it seems like an unlikely pattern
u && !u.p; // NOT OK
u && !u(); // NOT OK
});

3
javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.expected
View File

@@ -22,6 +22,9 @@
| UselessConditional.js:102:19:102:19 | x | This use of variable 'x' always evaluates to false. |
| UselessConditional.js:103:23:103:23 | x | This use of variable 'x' always evaluates to false. |
| UselessConditional.js:109:15:109:16 | {} | This expression always evaluates to true. |
| UselessConditional.js:129:6:129:24 | constantUndefined() | This call to constantUndefined always evaluates to false. |
| UselessConditional.js:135:6:135:32 | constan ... ined1() | This call to constantFalseOrUndefined1 always evaluates to false. |
| UselessConditional.js:139:6:139:32 | constan ... ined2() | This call to constantFalseOrUndefined2 always evaluates to false. |
| UselessConditionalGood.js:58:12:58:13 | x2 | This use of variable 'x2' always evaluates to false. |
| UselessConditionalGood.js:69:12:69:13 | xy | This use of variable 'xy' always evaluates to false. |
| UselessConditionalGood.js:85:12:85:13 | xy | This use of variable 'xy' always evaluates to false. |

31
javascript/ql/test/query-tests/Statements/UselessConditional/UselessConditional.js
View File

@@ -109,4 +109,35 @@ async function awaitFlow(){
if ((x && {}) || y) {} // NOT OK
});
(function(){
function constantFalse1() {
return false;
}
if (constantFalse1()) // OK
return;
function constantFalse2() {
return false;
}
let constantFalse = unknown? constantFalse1 : constantFalse2;
if (constantFalse2()) // OK
return;
function constantUndefined() {
return undefined;
}
if (constantUndefined()) // NOT OK
return;
function constantFalseOrUndefined1() {
return unknown? false: undefined;
}
if (constantFalseOrUndefined1()) // NOT OK
return;
let constantFalseOrUndefined2 = unknown? constantFalse1 : constantUndefined;
if (constantFalseOrUndefined2()) // NOT OK
return;
});
// semmle-extractor-options: --experimental

4
javascript/ql/test/query-tests/external/DuplicateToplevel/DuplicateToplevel.expected vendored
View File

@@ -1,2 +1,6 @@
| a.js:1:1:1:1 | <toplevel> | 90% of statements in this script are duplicated in $@. | e.js:1:1:1:1 | <toplevel> | another script |
| a.js:1:1:1:1 | <toplevel> | 100% of statements in this script are duplicated in $@. | b.js:1:1:1:1 | <toplevel> | another script |
| b.js:1:1:1:1 | <toplevel> | 90% of statements in this script are duplicated in $@. | e.js:1:1:1:1 | <toplevel> | another script |
| b.js:1:1:1:1 | <toplevel> | 100% of statements in this script are duplicated in $@. | a.js:1:1:1:1 | <toplevel> | another script |
| e.js:1:1:1:1 | <toplevel> | 90% of statements in this script are duplicated in $@. | a.js:1:1:1:1 | <toplevel> | another script |
| e.js:1:1:1:1 | <toplevel> | 90% of statements in this script are duplicated in $@. | b.js:1:1:1:1 | <toplevel> | another script |

3
javascript/ql/test/query-tests/external/DuplicateToplevel/a.js vendored
View File

@@ -7,4 +7,7 @@
arguments[0]--;
arguments[1] += 19;
arguments[0] * arguments[1];
arguments[2] / arguments[3];
arguments[4] % arguments[5];
arguments[6] % arguments[7];
}

3
javascript/ql/test/query-tests/external/DuplicateToplevel/b.js vendored
View File

@@ -7,4 +7,7 @@
arguments[0]--;
arguments[1] += 19;
arguments[0] * arguments[1];
arguments[2] / arguments[3];
arguments[4] % arguments[5];
arguments[6] % arguments[7];
}

13
javascript/ql/test/query-tests/external/DuplicateToplevel/e.js vendored Normal file
View File

@@ -0,0 +1,13 @@
{
if (arguments.length == 0)
23;
if (arguments.length % 2 != 0)
42;
console.log(arguments[0]);
arguments[0]--;
arguments[1] += 19;
arguments[0] * arguments[1];
arguments[2] / arguments[3];
arguments[4] % arguments[5];
/*arguments[6] % arguments[7]*/;
}