mirror of
https://github.com/github/codeql.git
synced 2026-05-05 05:35:13 +02:00
Merge pull request #11769 from erik-krogh/moreSan
JS: Sanitizer for `sanitizer(x) === true`
This commit is contained in:
Erik Krogh Kristensen
@@ -379,7 +379,10 @@ class NullLiteral extends @null_literal, Literal { }
|
||||
* false
|
||||
* ```
|
||||
*/
|
||||
class BooleanLiteral extends @boolean_literal, Literal { }
|
||||
class BooleanLiteral extends @boolean_literal, Literal {
|
||||
/** Gets the value of this literal. */
|
||||
boolean getBoolValue() { if this.getRawValue() = "true" then result = true else result = false }
|
||||
}
|
||||
|
||||
/**
|
||||
* A numeric literal.
|
||||
|
||||
@@ -161,7 +161,7 @@ abstract class Configuration extends string {
|
||||
*/
|
||||
predicate isBarrier(DataFlow::Node node) {
|
||||
exists(BarrierGuardNode guard |
|
||||
isBarrierGuardInternal(guard) and
|
||||
isBarrierGuardInternal(this, guard) and
|
||||
barrierGuardBlocksNode(guard, node, "")
|
||||
)
|
||||
}
|
||||
@@ -181,7 +181,7 @@ abstract class Configuration extends string {
|
||||
*/
|
||||
predicate isLabeledBarrier(DataFlow::Node node, FlowLabel lbl) {
|
||||
exists(BarrierGuardNode guard |
|
||||
isBarrierGuardInternal(guard) and
|
||||
isBarrierGuardInternal(this, guard) and
|
||||
barrierGuardBlocksNode(guard, node, lbl)
|
||||
)
|
||||
or
|
||||
@@ -198,17 +198,6 @@ abstract class Configuration extends string {
|
||||
*/
|
||||
predicate isBarrierGuard(BarrierGuardNode guard) { none() }
|
||||
|
||||
/**
|
||||
* Holds if `guard` is a barrier guard for this configuration, added through
|
||||
* `isBarrierGuard` or `AdditionalBarrierGuardNode`.
|
||||
*/
|
||||
pragma[nomagic]
|
||||
private predicate isBarrierGuardInternal(BarrierGuardNode guard) {
|
||||
isBarrierGuard(guard)
|
||||
or
|
||||
guard.(AdditionalBarrierGuardNode).appliesTo(this)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if data may flow from `source` to `sink` for this configuration.
|
||||
*/
|
||||
@@ -267,6 +256,17 @@ abstract class Configuration extends string {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `guard` is a barrier guard for this configuration, added through
|
||||
* `isBarrierGuard` or `AdditionalBarrierGuardNode`.
|
||||
*/
|
||||
pragma[nomagic]
|
||||
private predicate isBarrierGuardInternal(Configuration cfg, BarrierGuardNode guard) {
|
||||
cfg.isBarrierGuard(guard)
|
||||
or
|
||||
guard.(AdditionalBarrierGuardNode).appliesTo(cfg)
|
||||
}
|
||||
|
||||
/**
|
||||
* A label describing the kind of information tracked by a flow configuration.
|
||||
*
|
||||
@@ -368,6 +368,8 @@ private predicate barrierGuardBlocksExpr(
|
||||
// Handle labelled barrier guard functions specially, to avoid negative recursion
|
||||
// through the non-abstract 3-argument version of blocks().
|
||||
guard.(AdditionalBarrierGuardCall).internalBlocksLabel(outcome, test, label)
|
||||
or
|
||||
guard.(CallAgainstEqualityCheck).internalBlocksLabel(outcome, test, label)
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1979,7 +1981,7 @@ private class BarrierGuardFunction extends Function {
|
||||
/**
|
||||
* Holds if this function applies to the flow in `cfg`.
|
||||
*/
|
||||
predicate appliesTo(Configuration cfg) { cfg.isBarrierGuard(guard) }
|
||||
predicate appliesTo(Configuration cfg) { isBarrierGuardInternal(cfg, guard) }
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -1999,6 +2001,42 @@ private class AdditionalBarrierGuardCall extends AdditionalBarrierGuardNode, Dat
|
||||
override predicate appliesTo(Configuration cfg) { f.appliesTo(cfg) }
|
||||
}
|
||||
|
||||
/**
|
||||
* A sanitizer where an inner sanitizer is compared against a boolean.
|
||||
* E.g. (assuming `sanitizes(e)` is an existing sanitizer):
|
||||
* ```javascript
|
||||
* if (sanitizes(e) === true) {
|
||||
* // e is sanitized
|
||||
* }
|
||||
* ```
|
||||
*/
|
||||
private class CallAgainstEqualityCheck extends AdditionalBarrierGuardNode {
|
||||
DataFlow::BarrierGuardNode prev;
|
||||
boolean polarity;
|
||||
|
||||
CallAgainstEqualityCheck() {
|
||||
prev instanceof DataFlow::CallNode and
|
||||
exists(EqualityTest test, BooleanLiteral bool |
|
||||
this.asExpr() = test and
|
||||
test.hasOperands(prev.asExpr(), bool) and
|
||||
polarity = test.getPolarity().booleanXor(bool.getBoolValue())
|
||||
)
|
||||
}
|
||||
|
||||
override predicate blocks(boolean outcome, Expr e) {
|
||||
none() // handled by internalBlocksLabel
|
||||
}
|
||||
|
||||
predicate internalBlocksLabel(boolean outcome, Expr e, DataFlow::FlowLabel lbl) {
|
||||
exists(boolean prevOutcome |
|
||||
barrierGuardBlocksExpr(prev, prevOutcome, e, lbl) and
|
||||
outcome = prevOutcome.booleanXor(polarity)
|
||||
)
|
||||
}
|
||||
|
||||
override predicate appliesTo(Configuration cfg) { isBarrierGuardInternal(cfg, prev) }
|
||||
}
|
||||
|
||||
/**
|
||||
* A guard node for a variable in a negative condition, such as `x` in `if(!x)`.
|
||||
* Can be added to a `isBarrier` in a data-flow configuration to block flow through such checks.
|
||||
|
||||
@@ -301,6 +301,24 @@ nodes
|
||||
| lib/lib.js:562:26:562:29 | name |
|
||||
| lib/lib.js:566:26:566:29 | name |
|
||||
| lib/lib.js:566:26:566:29 | name |
|
||||
| lib/lib.js:572:41:572:44 | name |
|
||||
| lib/lib.js:572:41:572:44 | name |
|
||||
| lib/lib.js:573:22:573:25 | name |
|
||||
| lib/lib.js:573:22:573:25 | name |
|
||||
| lib/lib.js:579:25:579:28 | name |
|
||||
| lib/lib.js:579:25:579:28 | name |
|
||||
| lib/lib.js:590:29:590:32 | name |
|
||||
| lib/lib.js:590:29:590:32 | name |
|
||||
| lib/lib.js:593:25:593:28 | name |
|
||||
| lib/lib.js:593:25:593:28 | name |
|
||||
| lib/lib.js:608:42:608:45 | name |
|
||||
| lib/lib.js:608:42:608:45 | name |
|
||||
| lib/lib.js:609:22:609:25 | name |
|
||||
| lib/lib.js:609:22:609:25 | name |
|
||||
| lib/lib.js:626:29:626:32 | name |
|
||||
| lib/lib.js:626:29:626:32 | name |
|
||||
| lib/lib.js:629:25:629:28 | name |
|
||||
| lib/lib.js:629:25:629:28 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name |
|
||||
| lib/subLib2/compiled-file.ts:4:25:4:28 | name |
|
||||
@@ -703,6 +721,34 @@ edges
|
||||
| lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name |
|
||||
| lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name |
|
||||
| lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:573:22:573:25 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:573:22:573:25 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:573:22:573:25 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:573:22:573:25 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:579:25:579:28 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:579:25:579:28 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:579:25:579:28 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:579:25:579:28 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:590:29:590:32 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:590:29:590:32 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:590:29:590:32 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:590:29:590:32 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
|
||||
| lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
|
||||
| lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
|
||||
| lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name |
|
||||
@@ -826,6 +872,13 @@ edges
|
||||
| lib/lib.js:560:14:560:29 | "rm -rf " + name | lib/lib.js:558:41:558:44 | name | lib/lib.js:560:26:560:29 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:558:41:558:44 | name | library input | lib/lib.js:560:9:560:30 | exec("r ... + name) | shell command |
|
||||
| lib/lib.js:562:14:562:29 | "rm -rf " + name | lib/lib.js:558:41:558:44 | name | lib/lib.js:562:26:562:29 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:558:41:558:44 | name | library input | lib/lib.js:562:9:562:30 | exec("r ... + name) | shell command |
|
||||
| lib/lib.js:566:14:566:29 | "rm -rf " + name | lib/lib.js:558:41:558:44 | name | lib/lib.js:566:26:566:29 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:558:41:558:44 | name | library input | lib/lib.js:566:9:566:30 | exec("r ... + name) | shell command |
|
||||
| lib/lib.js:573:10:573:25 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:573:22:573:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:573:2:573:26 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:579:13:579:28 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:579:25:579:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:579:5:579:29 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:590:17:590:32 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:590:29:590:32 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:590:9:590:33 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:593:13:593:28 | "rm -rf " + name | lib/lib.js:572:41:572:44 | name | lib/lib.js:593:25:593:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:572:41:572:44 | name | library input | lib/lib.js:593:5:593:29 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:609:10:609:25 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:609:22:609:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:609:2:609:26 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:626:17:626:32 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:626:29:626:32 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:626:9:626:33 | cp.exec ... + name) | shell command |
|
||||
| lib/lib.js:629:13:629:28 | "rm -rf " + name | lib/lib.js:608:42:608:45 | name | lib/lib.js:629:25:629:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/lib.js:608:42:608:45 | name | library input | lib/lib.js:629:5:629:29 | cp.exec ... + name) | shell command |
|
||||
| lib/subLib2/compiled-file.ts:4:13:4:28 | "rm -rf " + name | lib/subLib2/compiled-file.ts:3:26:3:29 | name | lib/subLib2/compiled-file.ts:4:25:4:28 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/compiled-file.ts:3:26:3:29 | name | library input | lib/subLib2/compiled-file.ts:4:5:4:29 | cp.exec ... + name) | shell command |
|
||||
| lib/subLib2/special-file.js:4:10:4:25 | "rm -rf " + name | lib/subLib2/special-file.js:3:28:3:31 | name | lib/subLib2/special-file.js:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib2/special-file.js:3:28:3:31 | name | library input | lib/subLib2/special-file.js:4:2:4:26 | cp.exec ... + name) | shell command |
|
||||
| lib/subLib3/my-file.ts:4:10:4:25 | "rm -rf " + name | lib/subLib3/my-file.ts:3:28:3:31 | name | lib/subLib3/my-file.ts:4:22:4:25 | name | This string concatenation which depends on $@ is later used in a $@. | lib/subLib3/my-file.ts:3:28:3:31 | name | library input | lib/subLib3/my-file.ts:4:2:4:26 | cp.exec ... + name) | shell command |
|
||||
|
||||
@@ -568,3 +568,63 @@ module.exports.badSanitizer = function (name) {
|
||||
exec("rm -rf " + name); // OK
|
||||
}
|
||||
}
|
||||
|
||||
module.exports.safeWithBool = function (name) {
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
|
||||
if (isSafeName(name)) {
|
||||
cp.exec("rm -rf " + name); // OK
|
||||
}
|
||||
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
|
||||
if (isSafeName(name) === true) {
|
||||
cp.exec("rm -rf " + name); // OK
|
||||
}
|
||||
|
||||
if (isSafeName(name) !== false) {
|
||||
cp.exec("rm -rf " + name); // OK
|
||||
}
|
||||
|
||||
if (isSafeName(name) == false) {
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
}
|
||||
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
}
|
||||
|
||||
function indirectThing(name) {
|
||||
return isSafeName(name);
|
||||
}
|
||||
|
||||
function indirectThing2(name) {
|
||||
return isSafeName(name) === true;
|
||||
}
|
||||
|
||||
function moreIndirect(name) {
|
||||
return indirectThing2(name) !== false;
|
||||
}
|
||||
|
||||
module.exports.veryIndeirect = function (name) {
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
|
||||
if (indirectThing(name)) {
|
||||
cp.exec("rm -rf " + name); // OK
|
||||
}
|
||||
|
||||
if (indirectThing2(name)) {
|
||||
cp.exec("rm -rf " + name); // OK
|
||||
}
|
||||
|
||||
if (moreIndirect(name)) {
|
||||
cp.exec("rm -rf " + name); // OK
|
||||
}
|
||||
|
||||
if (moreIndirect(name) !== false) {
|
||||
cp.exec("rm -rf " + name); // OK
|
||||
} else {
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
}
|
||||
|
||||
cp.exec("rm -rf " + name); // NOT OK
|
||||
}
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
const Router = require('koa-router')
|
||||
const {Sequelize} = require("sequelize");
|
||||
|
||||
new Router().get("/hello", (ctx) => {
|
||||
const { version } = ctx.query;
|
||||
|
||||
if (version && validVersion(version) === false) {
|
||||
throw new Error(`invalid version ${version}`);
|
||||
}
|
||||
|
||||
const conditions = ['1'];
|
||||
|
||||
if (version) {
|
||||
conditions.push(`version = ${version}`)
|
||||
}
|
||||
|
||||
new Sequelize().query(`SELECT * FROM t WHERE ${conditions.join(' and ')}`, null); // OK
|
||||
});
|
||||
|
||||
function validVersion(version) {
|
||||
const pattern = /^[a-zA-Z0-9]+$/;
|
||||
return pattern.test(version);
|
||||
}
|
||||
Reference in New Issue
Block a user