Merge pull request #735 from asger-semmle/string-ops

Approved by xiemaisi
This commit is contained in:
semmle-qlci 2019-01-25 15:15:19 +00:00 коммит произвёл GitHub
Родитель fc00e0a64a 8294aeea74
Коммит d8947a71a5
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 4AEE18F83AFDEB23
14 изменённых файлов: 523 добавлений и 69 удалений

Просмотреть файл

@ -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

Просмотреть файл

@ -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)
}
}
}

Просмотреть файл

@ -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) {

Просмотреть файл

@ -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 |

Просмотреть файл

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

Просмотреть файл

@ -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)) {}
}

Просмотреть файл

@ -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 |

Просмотреть файл

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

Просмотреть файл

@ -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) {}
}

Просмотреть файл

@ -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 |

Просмотреть файл

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

Просмотреть файл

@ -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) {}
}

Просмотреть файл

@ -5,6 +5,7 @@
| 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 |

Просмотреть файл

@ -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
}
}