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

support interprocedural flow with custom load/store steps

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2020-01-15 13:48:16 +01:00
parent d09bce5cd7
commit 830100d2ed
7 changed files with 115 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -466,22 +466,22 @@ abstract class AdditionalFlowStep extends DataFlow::Node {
}
/**
* Holds if the `pred` should be stored in the object `succ` under the property `prop`.
* Holds if the `pred` should be stored in the object `succ` under the property `prop`.
*/
cached
predicate store(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
/**
* Holds if the property `prop` of the object `pred` should be loaded into `succ`.
* Holds if the property `prop` of the object `pred` should be loaded into `succ`.
*/
cached
predicate load(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
/**
* Holds if the property `prop` should be copied from the object `pred` to the object `succ`.
* Holds if the property `prop` should be copied from the object `pred` to the object `succ`.
*/
cached
predicate copyProperty(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
predicate copyProperty(DataFlow::Node pred, DataFlow::Node succ, string prop) { none() }
}
/**
@@ -584,7 +584,7 @@ private predicate exploratoryFlowStep(
basicFlowStep(pred, succ, _, cfg) or
basicStoreStep(pred, succ, _) or
basicLoadStep(pred, succ, _) or
any(AdditionalFlowStep s).store(pred, succ, _) or
cfg.isAdditionalStoreStep(pred, succ, _) or
any(AdditionalFlowStep s).load(pred, succ, _) or
@@ -765,6 +765,16 @@ private predicate storeStep(
(
returnedPropWrite(f, _, prop, mid)
or
exists(DataFlow::SourceNode base |
(
any(AdditionalFlowStep step).store(mid, _, prop)
or
cfg.isAdditionalStoreStep(mid, _, prop)
)
and
base.flowsToExpr(f.getAReturnedExpr())
)
or
succ instanceof DataFlow::NewNode and
receiverPropWrite(f, prop, mid)
)
@@ -775,12 +785,18 @@ private predicate storeStep(
* Holds if `f` may `read` property `prop` of parameter `parm`.
*/
private predicate parameterPropRead(
Function f, DataFlow::Node invk, DataFlow::Node arg, string prop, DataFlow::PropRead read,
Function f, DataFlow::Node invk, DataFlow::Node arg, string prop, DataFlow::Node read,
DataFlow::Configuration cfg
) {
exists(DataFlow::SourceNode parm |
exists(DataFlow::Node parm |
callInputStep(f, invk, arg, parm, cfg) and
read = parm.getAPropertyRead(prop)
(
read = parm.(DataFlow::SourceNode).getAPropertyRead(prop)
or
any(AdditionalFlowStep step).load(parm, read, prop)
or
cfg.isAdditionalLoadStep(parm, read, prop)
)
)
}
@@ -819,7 +835,7 @@ private predicate loadStep(
cfg.isAdditionalLoadStep(pred, succ, prop) and
summary = PathSummary::level()
or
exists(Function f, DataFlow::PropRead read |
exists(Function f, DataFlow::Node read |
parameterPropRead(f, succ, pred, prop, read, cfg) and
reachesReturn(f, read, cfg, summary)
)
@@ -866,17 +882,17 @@ private predicate flowThroughProperty(
/**
* Holds if the property `prop` is copied from `fromNode` to `toNode` using at least 1 step.
*
* The recursion of this predicate has been unfolded once compared to a naive implementation in order to avoid having no constraint on `prop`.
* Therefore a caller of this predicate should also test whether the `toNode` and `fromNode` are equal.
*
* The recursion of this predicate has been unfolded once compared to a naive implementation in order to avoid having no constraint on `prop`.
* Therefore a caller of this predicate should also test whether the `toNode` and `fromNode` are equal.
*/
private predicate existsCopyProperty(DataFlow::Node fromNode, DataFlow::Node toNode, string prop) {
exists(DataFlow::AdditionalFlowStep step, DataFlow::Node mid |
exists(DataFlow::AdditionalFlowStep step, DataFlow::Node mid |
step.copyProperty(fromNode, mid, prop) and
(
existsCopyProperty(mid, toNode, prop)
or
mid = toNode
mid = toNode
)
)
}

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

@@ -0,0 +1 @@
| tst.js:4:15:4:22 | "source" | tst.js:9:7:9:24 | readTaint(tainted) |

22
javascript/ql/test/library-tests/CustomLoadStoreSteps/test.ql Normal file
View File

@@ -0,0 +1,22 @@
import javascript
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "PromiseFlowTestingConfig" }
override predicate isSource(DataFlow::Node source) {
source.getEnclosingExpr().getStringValue() = "source"
}
override predicate isSink(DataFlow::Node sink) {
any(DataFlow::InvokeNode call | call.getCalleeName() = "sink").getAnArgument() = sink
}
// When the source code states that "foo" is being read, "bar" is additionally being read.
override predicate isAdditionalLoadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
pred.(DataFlow::SourceNode).getAPropertyRead("foo") = succ and prop = "bar"
}
}
from DataFlow::Node pred, DataFlow::Node succ, Configuration cfg
where cfg.hasFlow(pred, succ)
select pred, succ

10
javascript/ql/test/library-tests/CustomLoadStoreSteps/tst.js Normal file
View File

@@ -0,0 +1,10 @@
// When the source code states that "foo" is being read, "bar" is additionally being read.
(function () {
var source = "source";
var tainted = { bar: source };
function readTaint(x) {
return x.foo;
}
sink(readTaint(tainted));
})();

21
javascript/ql/test/library-tests/Promises/flow.js
View File

@@ -13,7 +13,7 @@
var p4 = new Promise((resolve, reject) => reject(source));
try {
var foo = await p4;
} catch(e) {
} catch (e) {
sink(e); // NOT OK!
}
@@ -31,19 +31,24 @@
new Promise((resolve, reject) => reject(source)).catch(x => sink(x)); // NOT OK!
Promise.resolve(source).catch(() => {}).then(a => sink(a)); // NOT OK!
Promise.resolve(source).catch(() => { }).then(a => sink(a)); // NOT OK!
var p5 = Promise.resolve(source);
var p6 = p5.catch(() => {});
var p6 = p5.catch(() => { });
var p7 = p6.then(a => sink(a)); // NOT OK!
new Promise((resolve, reject) => reject(source)).then(() => {}).catch(x => sink(x)); // NOT OK!
new Promise((resolve, reject) => reject(source)).then(() => { }).catch(x => sink(x)); // NOT OK!
new Promise((resolve, reject) => reject(source)).then(() => {}, () => {}).catch(x => sink(x)); // OK!
new Promise((resolve, reject) => reject(source)).then(() => { }, () => { }).catch(x => sink(x)); // OK!
Promise.resolve(source).catch(() => {}).catch(() => {}).catch(() => {}).then(a => sink(a)); // NOT OK!
Promise.resolve(source).catch(() => { }).catch(() => { }).catch(() => { }).then(a => sink(a)); // NOT OK!
Promise.resolve(source).finally(() => {}).then(a => sink(a)); // NOT OK!
Promise.resolve(source).finally(() => { }).then(a => sink(a)); // NOT OK!
new Promise(() => {throw source}).catch(x => sink(x)); // NOT OK!
new Promise(() => { throw source }).catch(x => sink(x)); // NOT OK!
function createPromise(src) {
return Promise.resolve(src);
}
createPromise(source).then(v => sink(v)); // NOT OK!
})();

20
javascript/ql/test/library-tests/Promises/interflow.js Normal file
View File

@@ -0,0 +1,20 @@
(function () {
function getSource() {
var source = "source"; // step 1
return source; // step 2
}
loadScript(getSource()) // step 3
.then(function () { })
.then(function () { })
.catch(handleError);
function loadScript(src) { // step 4 (is summarized)
return new Promise(function (resolve, reject) {
setTimeout(function (error) {
reject(new Error('Blah: ' + src)); // step 5
}, 1000);
});
}
function handleError(error) { // step 6
sink(error); // step 7
}
})();

31
javascript/ql/test/library-tests/Promises/tests.expected
View File

@@ -8,14 +8,15 @@ test_ResolvedPromiseDefinition
| flow.js:36:11:36:33 | Promise ... source) | flow.js:36:27:36:32 | source |
| flow.js:44:2:44:24 | Promise ... source) | flow.js:44:18:44:23 | source |
| flow.js:46:2:46:24 | Promise ... source) | flow.js:46:18:46:23 | source |
| flow.js:51:10:51:29 | Promise.resolve(src) | flow.js:51:26:51:28 | src |
| promises.js:53:19:53:41 | Promise ... source) | promises.js:53:35:53:40 | source |
| promises.js:62:19:62:41 | Promise ... source) | promises.js:62:35:62:40 | source |
| promises.js:71:5:71:27 | Promise ... source) | promises.js:71:21:71:26 | source |
test_PromiseDefinition_getARejectHandler
| flow.js:26:2:26:49 | new Pro ... ource)) | flow.js:26:69:26:80 | y => sink(y) |
| flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:57:32:68 | x => sink(x) |
| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:66:42:73 | () => {} |
| flow.js:48:2:48:34 | new Pro ... ource}) | flow.js:48:42:48:53 | x => sink(x) |
| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:67:42:75 | () => { } |
| flow.js:48:2:48:36 | new Pro ... urce }) | flow.js:48:44:48:55 | x => sink(x) |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:20:6:22:3 | (v) => ... v;\\n } |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:23:18:25:3 | (v) => ... v;\\n } |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:26:20:28:3 | (v) => ... v;\\n } |
@@ -28,7 +29,8 @@ test_PromiseDefinition_getExecutor
| flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:14:32:48 | (resolv ... source) |
| flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:14:40:48 | (resolv ... source) |
| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:14:42:48 | (resolv ... source) |
| flow.js:48:2:48:34 | new Pro ... ource}) | flow.js:48:14:48:33 | () => {throw source} |
| flow.js:48:2:48:36 | new Pro ... urce }) | flow.js:48:14:48:35 | () => { ... ource } |
| interflow.js:11:12:15:6 | new Pro ... \\n }) | interflow.js:11:24:15:5 | functio ... ;\\n } |
| promises.js:3:17:5:4 | new Pro ... );\\n }) | promises.js:3:29:5:3 | functio ... e);\\n } |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:10:30:17:3 | (res, r ... e);\\n } |
| promises.js:33:19:35:6 | new Pro ... \\n }) | promises.js:33:31:35:5 | functio ... ;\\n } |
@@ -44,7 +46,8 @@ test_PromiseDefinition
| flow.js:32:2:32:49 | new Pro ... ource)) |
| flow.js:40:2:40:49 | new Pro ... ource)) |
| flow.js:42:2:42:49 | new Pro ... ource)) |
| flow.js:48:2:48:34 | new Pro ... ource}) |
| flow.js:48:2:48:36 | new Pro ... urce }) |
| interflow.js:11:12:15:6 | new Pro ... \\n }) |
| promises.js:3:17:5:4 | new Pro ... );\\n }) |
| promises.js:10:18:17:4 | new Pro ... );\\n }) |
| promises.js:33:19:35:6 | new Pro ... \\n }) |
@@ -52,8 +55,8 @@ test_PromiseDefinition
test_PromiseDefinition_getAResolveHandler
| flow.js:24:2:24:49 | new Pro ... ource)) | flow.js:24:56:24:67 | x => sink(x) |
| flow.js:26:2:26:49 | new Pro ... ource)) | flow.js:26:56:26:66 | x => foo(x) |
| flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:56:40:63 | () => {} |
| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:56:42:63 | () => {} |
| flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:56:40:64 | () => { } |
| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:56:42:64 | () => { } |
| promises.js:3:17:5:4 | new Pro ... );\\n }) | promises.js:6:16:8:3 | functio ... al;\\n } |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:18:17:20:3 | (v) => ... v;\\n } |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:26:20:28:3 | (v) => ... v;\\n } |
@@ -68,6 +71,7 @@ test_PromiseDefinition_getRejectParameter
| flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:24:32:29 | reject |
| flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:24:40:29 | reject |
| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:24:42:29 | reject |
| interflow.js:11:12:15:6 | new Pro ... \\n }) | interflow.js:11:43:11:48 | reject |
| promises.js:3:17:5:4 | new Pro ... );\\n }) | promises.js:3:48:3:53 | reject |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:10:36:10:38 | rej |
| promises.js:33:19:35:6 | new Pro ... \\n }) | promises.js:33:50:33:55 | reject |
@@ -81,13 +85,14 @@ test_PromiseDefinition_getResolveParameter
| flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:15:32:21 | resolve |
| flow.js:40:2:40:49 | new Pro ... ource)) | flow.js:40:15:40:21 | resolve |
| flow.js:42:2:42:49 | new Pro ... ource)) | flow.js:42:15:42:21 | resolve |
| interflow.js:11:12:15:6 | new Pro ... \\n }) | interflow.js:11:34:11:40 | resolve |
| promises.js:3:17:5:4 | new Pro ... );\\n }) | promises.js:3:39:3:45 | resolve |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:10:31:10:33 | res |
| promises.js:33:19:35:6 | new Pro ... \\n }) | promises.js:33:41:33:47 | resolve |
| promises.js:43:19:45:6 | Q.Promi ... \\n }) | promises.js:43:39:43:45 | resolve |
test_PromiseDefinition_getACatchHandler
| flow.js:32:2:32:49 | new Pro ... ource)) | flow.js:32:57:32:68 | x => sink(x) |
| flow.js:48:2:48:34 | new Pro ... ource}) | flow.js:48:42:48:53 | x => sink(x) |
| flow.js:48:2:48:36 | new Pro ... urce }) | flow.js:48:44:48:55 | x => sink(x) |
| promises.js:10:18:17:4 | new Pro ... );\\n }) | promises.js:23:18:25:3 | (v) => ... v;\\n } |
flow
| flow.js:2:15:2:22 | "source" | flow.js:5:7:5:14 | await p1 |
@@ -97,9 +102,11 @@ flow
| flow.js:2:15:2:22 | "source" | flow.js:26:79:26:79 | y |
| flow.js:2:15:2:22 | "source" | flow.js:28:58:28:58 | z |
| flow.js:2:15:2:22 | "source" | flow.js:32:67:32:67 | x |
| flow.js:2:15:2:22 | "source" | flow.js:34:57:34:57 | a |
| flow.js:2:15:2:22 | "source" | flow.js:34:58:34:58 | a |
| flow.js:2:15:2:22 | "source" | flow.js:38:29:38:29 | a |
| flow.js:2:15:2:22 | "source" | flow.js:40:82:40:82 | x |
| flow.js:2:15:2:22 | "source" | flow.js:44:89:44:89 | a |
| flow.js:2:15:2:22 | "source" | flow.js:46:59:46:59 | a |
| flow.js:2:15:2:22 | "source" | flow.js:48:52:48:52 | x |
| flow.js:2:15:2:22 | "source" | flow.js:40:83:40:83 | x |
| flow.js:2:15:2:22 | "source" | flow.js:44:92:44:92 | a |
| flow.js:2:15:2:22 | "source" | flow.js:46:60:46:60 | a |
| flow.js:2:15:2:22 | "source" | flow.js:48:54:48:54 | x |
| flow.js:2:15:2:22 | "source" | flow.js:53:39:53:39 | v |
| interflow.js:3:18:3:25 | "source" | interflow.js:18:10:18:14 | error |