From 398ee0c13363d3ef280fd3107a713668f3f3b747 Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Fri, 7 Jun 2019 14:33:26 +0100 Subject: [PATCH 1/9] JavaScript: Add tests for data-flow tutorial. --- .../Global data flow/query1.expected | 3 ++ .../Global data flow/query1.ql | 17 +++++++++++ .../Global data flow/query2.expected | 2 ++ .../Global data flow/query2.ql | 21 +++++++++++++ .../Global data flow/query3.expected | 2 ++ .../Global data flow/query3.ql | 30 +++++++++++++++++++ .../Global data flow/query4.expected | 4 +++ .../Global data flow/query4.ql | 25 ++++++++++++++++ .../Global data flow/query5.expected | 4 +++ .../Global data flow/query5.ql | 26 ++++++++++++++++ .../Global data flow/test1.js | 13 ++++++++ .../Global data flow/test2.js | 20 +++++++++++++ .../Global data flow/test3.js | 19 ++++++++++++ .../Global data flow/test4.js | 14 +++++++++ .../Local data flow/query1.expected | 3 ++ .../Local data flow/query1.ql | 7 +++++ .../Local data flow/query2.expected | 1 + .../Local data flow/query2.ql | 3 ++ .../Local data flow/query3.expected | 1 + .../Local data flow/query3.ql | 3 ++ .../Local data flow/query4.expected | 1 + .../Local data flow/query4.ql | 8 +++++ .../Local data flow/test.js | 8 +++++ 23 files changed, 235 insertions(+) create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query1.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query1.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query2.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query2.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query3.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query3.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test1.js create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test2.js create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test3.js create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test4.js create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query1.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query1.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query2.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query2.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query3.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query3.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query4.expected create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query4.ql create mode 100644 javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/test.js diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query1.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query1.expected new file mode 100644 index 00000000000..472b4779d39 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query1.expected @@ -0,0 +1,3 @@ +| test1.js:13:16:13:30 | process.argv[2] | test1.js:6:15:6:15 | p | +| test2.js:20:16:20:30 | process.argv[2] | test2.js:13:15:13:15 | p | +| test3.js:19:16:19:30 | process.argv[2] | test3.js:12:15:12:15 | p | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query1.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query1.ql new file mode 100644 index 00000000000..68828b0005d --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query1.ql @@ -0,0 +1,17 @@ +import javascript + +class CommandLineFileNameConfiguration extends TaintTracking::Configuration { + CommandLineFileNameConfiguration() { this = "CommandLineFileNameConfiguration" } + + override predicate isSource(DataFlow::Node source) { + DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead() = source + } + + override predicate isSink(DataFlow::Node sink) { + DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink + } +} + +from CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink +where cfg.hasFlow(source, sink) +select source, sink diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query2.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query2.expected new file mode 100644 index 00000000000..490fc7a8d27 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query2.expected @@ -0,0 +1,2 @@ +| test1.js:13:16:13:30 | process.argv[2] | test1.js:6:15:6:15 | p | +| test3.js:19:16:19:30 | process.argv[2] | test3.js:12:15:12:15 | p | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query2.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query2.ql new file mode 100644 index 00000000000..f94ae08309f --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query2.ql @@ -0,0 +1,21 @@ +import javascript + +class CommandLineFileNameConfiguration extends TaintTracking::Configuration { + CommandLineFileNameConfiguration() { this = "CommandLineFileNameConfiguration" } + + override predicate isSource(DataFlow::Node source) { + DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead() = source + } + + override predicate isSink(DataFlow::Node sink) { + DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink + } + + override predicate isSanitizer(DataFlow::Node nd) { + nd.(DataFlow::CallNode).getCalleeName() = "checkPath" + } +} + +from CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink +where cfg.hasFlow(source, sink) +select source, sink diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query3.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query3.expected new file mode 100644 index 00000000000..8e625f94ee0 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query3.expected @@ -0,0 +1,2 @@ +| test1.js:13:16:13:30 | process.argv[2] | test1.js:6:15:6:15 | p | +| test2.js:20:16:20:30 | process.argv[2] | test2.js:13:15:13:15 | p | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query3.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query3.ql new file mode 100644 index 00000000000..8fe2e962849 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query3.ql @@ -0,0 +1,30 @@ +import javascript + +class CheckPathSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode { + CheckPathSanitizerGuard() { this.getCalleeName() = "checkPath" } + + override predicate sanitizes(boolean outcome, Expr e) { + outcome = true and + e = getArgument(0).asExpr() + } +} + +class CommandLineFileNameConfiguration extends TaintTracking::Configuration { + CommandLineFileNameConfiguration() { this = "CommandLineFileNameConfiguration" } + + override predicate isSource(DataFlow::Node source) { + DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead() = source + } + + override predicate isSink(DataFlow::Node sink) { + DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink + } + + override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) { + nd instanceof CheckPathSanitizerGuard + } +} + +from CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink +where cfg.hasFlow(source, sink) +select source, sink diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.expected new file mode 100644 index 00000000000..20f2278686a --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.expected @@ -0,0 +1,4 @@ +| test1.js:13:16:13:30 | process.argv[2] | test1.js:6:15:6:15 | p | +| test2.js:20:16:20:30 | process.argv[2] | test2.js:13:15:13:15 | p | +| test3.js:19:16:19:30 | process.argv[2] | test3.js:12:15:12:15 | p | +| test4.js:14:16:14:30 | process.argv[2] | test4.js:7:13:7:13 | p | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.ql new file mode 100644 index 00000000000..3a224bab804 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query4.ql @@ -0,0 +1,25 @@ +import javascript + +class CommandLineFileNameConfiguration extends TaintTracking::Configuration { + CommandLineFileNameConfiguration() { this = "CommandLineFileNameConfiguration" } + + override predicate isSource(DataFlow::Node source) { + DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead() = source + } + + override predicate isSink(DataFlow::Node sink) { + DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink + } + + override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode c | + c = DataFlow::moduleImport("resolve-symlinks").getACall() and + pred = c.getArgument(0) and + succ = c + ) + } +} + +from CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink +where cfg.hasFlow(source, sink) +select source, sink diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.expected new file mode 100644 index 00000000000..20f2278686a --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.expected @@ -0,0 +1,4 @@ +| test1.js:13:16:13:30 | process.argv[2] | test1.js:6:15:6:15 | p | +| test2.js:20:16:20:30 | process.argv[2] | test2.js:13:15:13:15 | p | +| test3.js:19:16:19:30 | process.argv[2] | test3.js:12:15:12:15 | p | +| test4.js:14:16:14:30 | process.argv[2] | test4.js:7:13:7:13 | p | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql new file mode 100644 index 00000000000..ee55f91d23e --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/query5.ql @@ -0,0 +1,26 @@ +import javascript + +class StepThroughResolveSymlinks extends TaintTracking::AdditionalTaintStep, DataFlow::CallNode { + StepThroughResolveSymlinks() { this = DataFlow::moduleImport("resolve-symlinks").getACall() } + + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + pred = this.getArgument(0) and + succ = this + } +} + +class CommandLineFileNameConfiguration extends TaintTracking::Configuration { + CommandLineFileNameConfiguration() { this = "CommandLineFileNameConfiguration" } + + override predicate isSource(DataFlow::Node source) { + DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead() = source + } + + override predicate isSink(DataFlow::Node sink) { + DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink + } +} + +from CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink +where cfg.hasFlow(source, sink) +select source, sink diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test1.js b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test1.js new file mode 100644 index 00000000000..b9958d7d918 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test1.js @@ -0,0 +1,13 @@ +const fs = require('fs'), + path = require('path'); + +function readFileHelper(p) { // #2 + p = path.resolve(p); // #3 + fs.readFile(p, // #4 + 'utf8', (err, data) => { + if (err) throw err; + console.log(data); + }); +} + +readFileHelper(process.argv[2]); // #1 diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test2.js b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test2.js new file mode 100644 index 00000000000..63541ea7eec --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test2.js @@ -0,0 +1,20 @@ +const fs = require('fs'), + path = require('path'); + +function checkPath(p) { + p = path.resolve(p); + if (!p.startsWith(process.cwd() + path.sep)) + throw new Error("Invalid path " + p); + return p; +} + +function readFileHelper(p) { + p = checkPath(p); + fs.readFile(p, + 'utf8', (err, data) => { + if (err) throw err; + console.log(data); + }); +} + +readFileHelper(process.argv[2]); diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test3.js b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test3.js new file mode 100644 index 00000000000..9ca228de0c6 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test3.js @@ -0,0 +1,19 @@ +const fs = require('fs'), + path = require('path'); + +function checkPath(p) { + p = path.resolve(p); + return p.startsWith(process.cwd() + path.sep); +} + +function readFileHelper(p) { + if (!checkPath(p)) + return; + fs.readFile(p, + 'utf8', (err, data) => { + if (err) throw err; + console.log(data); + }); +} + +readFileHelper(process.argv[2]); diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test4.js b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test4.js new file mode 100644 index 00000000000..d0015356de9 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Global data flow/test4.js @@ -0,0 +1,14 @@ +const fs = require('fs'), +path = require('path'), +resolveSymlinks = require('resolve-symlinks'); + +function readFileHelper(p) { +p = resolveSymlinks(p); +fs.readFile(p, +'utf8', (err, data) => { +if (err) throw err; +console.log(data); +}); +} + +readFileHelper(process.argv[2]); diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query1.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query1.expected new file mode 100644 index 00000000000..42bfa5e6430 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query1.expected @@ -0,0 +1,3 @@ +| test.js:4:5:4:22 | firstArg | +| test.js:4:16:4:22 | args[2] | +| test.js:5:13:5:20 | firstArg | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query1.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query1.ql new file mode 100644 index 00000000000..2351c82d112 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query1.ql @@ -0,0 +1,7 @@ +import javascript + +from DataFlow::MethodCallNode readFile, DataFlow::Node source +where + readFile.getMethodName() = "readFile" and + source.getASuccessor*() = readFile.getArgument(0) +select source diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query2.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query2.expected new file mode 100644 index 00000000000..5a8463cc336 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query2.expected @@ -0,0 +1 @@ +| test.js:4:16:4:22 | args[2] | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query2.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query2.ql new file mode 100644 index 00000000000..c5fa9ec79fd --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query2.ql @@ -0,0 +1,3 @@ +import javascript + +select DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyReference() diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query3.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query3.expected new file mode 100644 index 00000000000..3d5651008ab --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query3.expected @@ -0,0 +1 @@ +| test.js:5:1:8:2 | fs.read ... ta);\\n}) | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query3.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query3.ql new file mode 100644 index 00000000000..5c12b062e6b --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query3.ql @@ -0,0 +1,3 @@ +import javascript + +select DataFlow::moduleMember("fs", "readFile").getACall() diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query4.expected b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query4.expected new file mode 100644 index 00000000000..b9d35741691 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query4.expected @@ -0,0 +1 @@ +| test.js:4:16:4:22 | args[2] | test.js:5:1:8:2 | fs.read ... ta);\\n}) | diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query4.ql b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query4.ql new file mode 100644 index 00000000000..bea4bd0f559 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/query4.ql @@ -0,0 +1,8 @@ +import javascript + +from DataFlow::SourceNode arg, DataFlow::CallNode call +where + arg = DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyReference() and + call = DataFlow::moduleMember("fs", "readFile").getACall() and + arg.flowsTo(call.getArgument(0)) +select arg, call diff --git a/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/test.js b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/test.js new file mode 100644 index 00000000000..4ee1e7fb775 --- /dev/null +++ b/javascript/ql/test/tutorials/Analyzing data flow in JavaScript/Local data flow/test.js @@ -0,0 +1,8 @@ +var fs = require('fs'); + +var args = process.argv; +var firstArg = args[2]; +fs.readFile(firstArg, 'utf8', (err, data) => { + if (err) throw err; + console.log(data); +}); \ No newline at end of file From 0f0dc812911eb50cf1967aa2e9b3213ee850c847 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Thu, 6 Jun 2019 12:52:27 +0100 Subject: [PATCH 2/9] Python ESSA: Remove method-callsite definitions when call is in a test defining a pi-node. --- .../ql/src/semmle/python/dataflow/SsaDefinitions.qll | 3 ++- .../library-tests/PointsTo/new/Dataflow.expected | 8 ++++++++ .../library-tests/PointsTo/new/Definitions.expected | 8 ++++++++ .../ql/test/library-tests/PointsTo/new/Live.expected | 12 ++++++++++++ .../library-tests/PointsTo/new/NameSpace.expected | 1 + .../PointsTo/new/PointsToUnknown.expected | 7 +++++++ .../PointsTo/new/PointsToWithContext.expected | 2 ++ .../PointsTo/new/PointsToWithType.expected | 2 ++ .../ql/test/library-tests/PointsTo/new/SSA.expected | 1 + .../PointsTo/new/SourceEdgeDefinitions.expected | 1 + .../PointsTo/new/SourceNodeDefinitions.expected | 5 +++++ .../test/library-tests/PointsTo/new/SsaUses.expected | 8 ++++++++ .../test/library-tests/PointsTo/new/Values.expected | 1 + .../test/library-tests/PointsTo/new/VarUses.expected | 10 ++++++++++ .../library-tests/PointsTo/new/code/b_condition.py | 7 +++++++ 15 files changed, 75 insertions(+), 1 deletion(-) diff --git a/python/ql/src/semmle/python/dataflow/SsaDefinitions.qll b/python/ql/src/semmle/python/dataflow/SsaDefinitions.qll index 9ecb1c550e7..b2f80189327 100644 --- a/python/ql/src/semmle/python/dataflow/SsaDefinitions.qll +++ b/python/ql/src/semmle/python/dataflow/SsaDefinitions.qll @@ -356,7 +356,8 @@ cached module SsaSource { /** Holds if `v` is used as the receiver in a method call. */ cached predicate method_call_refinement(Variable v, ControlFlowNode use, CallNode call) { use = v.getAUse() and - call.getFunction().(AttrNode).getObject() = use + call.getFunction().(AttrNode).getObject() = use and + not test_contains(_, call) } /** Holds if `v` is defined by assignment at `defn` and given `value`. */ diff --git a/python/ql/test/library-tests/PointsTo/new/Dataflow.expected b/python/ql/test/library-tests/PointsTo/new/Dataflow.expected index 193ccf8fd1e..e643bccb3d9 100644 --- a/python/ql/test/library-tests/PointsTo/new/Dataflow.expected +++ b/python/ql/test/library-tests/PointsTo/new/Dataflow.expected @@ -54,6 +54,7 @@ | b_condition.py:0 | h_0 = ScopeEntryDefinition | | b_condition.py:0 | k_0 = ScopeEntryDefinition | | b_condition.py:0 | loop_0 = ScopeEntryDefinition | +| b_condition.py:0 | method_check_0 = ScopeEntryDefinition | | b_condition.py:0 | not_or_not_0 = ScopeEntryDefinition | | b_condition.py:0 | odasa6261_0 = ScopeEntryDefinition | | b_condition.py:0 | split_bool1_0 = ScopeEntryDefinition | @@ -160,6 +161,13 @@ | b_condition.py:104 | a_1 = Pi(a_0) [false] | | b_condition.py:105 | a_2 = Pi(a_1) [false] | | b_condition.py:107 | a_3 = Pi(a_2) [false] | +| b_condition.py:109 | method_check_1 = FunctionExpr | +| b_condition.py:109 | x_0 = ParameterDefinition | +| b_condition.py:109 | x_5 = phi(x_2, x_4) | +| b_condition.py:111 | x_1 = Pi(x_0) [true] | +| b_condition.py:111 | x_2 = ArgumentRefinement(x_1) | +| b_condition.py:113 | x_3 = Pi(x_0) [false] | +| b_condition.py:113 | x_4 = ArgumentRefinement(x_3) | | d_globals.py:0 | D_0 = ScopeEntryDefinition | | d_globals.py:0 | Ugly_0 = ScopeEntryDefinition | | d_globals.py:0 | X_0 = ScopeEntryDefinition | diff --git a/python/ql/test/library-tests/PointsTo/new/Definitions.expected b/python/ql/test/library-tests/PointsTo/new/Definitions.expected index 3cce615988f..5d519fa01cb 100644 --- a/python/ql/test/library-tests/PointsTo/new/Definitions.expected +++ b/python/ql/test/library-tests/PointsTo/new/Definitions.expected @@ -37,6 +37,7 @@ | b_condition.py:0 | Global Variable h | ScopeEntryDefinition | | b_condition.py:0 | Global Variable k | ScopeEntryDefinition | | b_condition.py:0 | Global Variable loop | ScopeEntryDefinition | +| b_condition.py:0 | Global Variable method_check | ScopeEntryDefinition | | b_condition.py:0 | Global Variable not_or_not | ScopeEntryDefinition | | b_condition.py:0 | Global Variable odasa6261 | ScopeEntryDefinition | | b_condition.py:0 | Global Variable split_bool1 | ScopeEntryDefinition | @@ -136,6 +137,13 @@ | b_condition.py:104 | Local Variable a | PyEdgeRefinement | | b_condition.py:105 | Local Variable a | PyEdgeRefinement | | b_condition.py:107 | Local Variable a | PyEdgeRefinement | +| b_condition.py:109 | Global Variable method_check | AssignmentDefinition | +| b_condition.py:109 | Local Variable x | ParameterDefinition | +| b_condition.py:109 | Local Variable x | PhiFunction | +| b_condition.py:111 | Local Variable x | ArgumentRefinement | +| b_condition.py:111 | Local Variable x | PyEdgeRefinement | +| b_condition.py:113 | Local Variable x | ArgumentRefinement | +| b_condition.py:113 | Local Variable x | PyEdgeRefinement | | d_globals.py:0 | Global Variable D | ScopeEntryDefinition | | d_globals.py:0 | Global Variable Ugly | ScopeEntryDefinition | | d_globals.py:0 | Global Variable X | ScopeEntryDefinition | diff --git a/python/ql/test/library-tests/PointsTo/new/Live.expected b/python/ql/test/library-tests/PointsTo/new/Live.expected index a0a70d136c7..8717de94032 100644 --- a/python/ql/test/library-tests/PointsTo/new/Live.expected +++ b/python/ql/test/library-tests/PointsTo/new/Live.expected @@ -162,6 +162,8 @@ | Global Variable list | b_condition.py:101 | entry | | Global Variable loop | b_condition.py:42 | exit | | Global Variable loop | b_condition.py:43 | entry | +| Global Variable method_check | b_condition.py:42 | exit | +| Global Variable method_check | b_condition.py:43 | entry | | Global Variable not_or_not | b_condition.py:42 | exit | | Global Variable not_or_not | b_condition.py:43 | entry | | Global Variable object | b_condition.py:0 | entry | @@ -298,6 +300,10 @@ | Global Variable use | b_condition.py:88 | exit | | Global Variable use | b_condition.py:90 | entry | | Global Variable use | b_condition.py:90 | exit | +| Global Variable use | b_condition.py:109 | entry | +| Global Variable use | b_condition.py:110 | exit | +| Global Variable use | b_condition.py:111 | entry | +| Global Variable use | b_condition.py:113 | entry | | Global Variable v2 | b_condition.py:42 | exit | | Global Variable v2 | b_condition.py:43 | entry | | Global Variable v2 | b_condition.py:44 | exit | @@ -372,6 +378,12 @@ | Local Variable x | b_condition.py:88 | exit | | Local Variable x | b_condition.py:90 | entry | | Local Variable x | b_condition.py:90 | exit | +| Local Variable x | b_condition.py:109 | entry | +| Local Variable x | b_condition.py:110 | exit | +| Local Variable x | b_condition.py:111 | entry | +| Local Variable x | b_condition.py:111 | exit | +| Local Variable x | b_condition.py:113 | entry | +| Local Variable x | b_condition.py:113 | exit | | Local Variable y | b_condition.py:5 | entry | | Local Variable y | b_condition.py:5 | exit | | Local Variable y | b_condition.py:7 | exit | diff --git a/python/ql/test/library-tests/PointsTo/new/NameSpace.expected b/python/ql/test/library-tests/PointsTo/new/NameSpace.expected index bcf7d969dd7..b3c8ad622a5 100644 --- a/python/ql/test/library-tests/PointsTo/new/NameSpace.expected +++ b/python/ql/test/library-tests/PointsTo/new/NameSpace.expected @@ -14,6 +14,7 @@ | b_condition.py:0 | Module code.b_condition | h | Function h | | b_condition.py:0 | Module code.b_condition | k | Function k | | b_condition.py:0 | Module code.b_condition | loop | Function loop | +| b_condition.py:0 | Module code.b_condition | method_check | Function method_check | | b_condition.py:0 | Module code.b_condition | not_or_not | Function not_or_not | | b_condition.py:0 | Module code.b_condition | odasa6261 | Function odasa6261 | | b_condition.py:0 | Module code.b_condition | split_bool1 | Function split_bool1 | diff --git a/python/ql/test/library-tests/PointsTo/new/PointsToUnknown.expected b/python/ql/test/library-tests/PointsTo/new/PointsToUnknown.expected index 93f0b8c515c..fe490094107 100644 --- a/python/ql/test/library-tests/PointsTo/new/PointsToUnknown.expected +++ b/python/ql/test/library-tests/PointsTo/new/PointsToUnknown.expected @@ -114,6 +114,13 @@ | b_condition.py:99 | ControlFlowNode for use() | 99 | | b_condition.py:105 | ControlFlowNode for Subscript | 105 | | b_condition.py:105 | ControlFlowNode for UnaryExpr | 105 | +| b_condition.py:110 | ControlFlowNode for Attribute | 110 | +| b_condition.py:110 | ControlFlowNode for Attribute() | 110 | +| b_condition.py:110 | ControlFlowNode for x | 109 | +| b_condition.py:111 | ControlFlowNode for use | 111 | +| b_condition.py:111 | ControlFlowNode for use() | 111 | +| b_condition.py:113 | ControlFlowNode for use | 113 | +| b_condition.py:113 | ControlFlowNode for use() | 113 | | c_tests.py:5 | ControlFlowNode for IfExp | 5 | | c_tests.py:5 | ControlFlowNode for cond | 5 | | c_tests.py:5 | ControlFlowNode for unknown | 5 | diff --git a/python/ql/test/library-tests/PointsTo/new/PointsToWithContext.expected b/python/ql/test/library-tests/PointsTo/new/PointsToWithContext.expected index b7cced59a58..5b4b438bbab 100755 --- a/python/ql/test/library-tests/PointsTo/new/PointsToWithContext.expected +++ b/python/ql/test/library-tests/PointsTo/new/PointsToWithContext.expected @@ -185,6 +185,8 @@ WARNING: Predicate points_to has been deprecated and may be removed in future (P | b_condition.py:106 | ControlFlowNode for Exception | builtin-class Exception | builtin-class type | 106 | runtime | | b_condition.py:106 | ControlFlowNode for Exception() | Exception() | builtin-class Exception | 106 | runtime | | b_condition.py:107 | ControlFlowNode for Str | 'Hello' | builtin-class str | 107 | runtime | +| b_condition.py:109 | ControlFlowNode for FunctionExpr | Function method_check | builtin-class function | 109 | import | +| b_condition.py:109 | ControlFlowNode for method_check | Function method_check | builtin-class function | 109 | import | | e_temporal.py:2 | ControlFlowNode for ImportExpr | Module sys | builtin-class module | 2 | import | | e_temporal.py:2 | ControlFlowNode for sys | Module sys | builtin-class module | 2 | import | | e_temporal.py:4 | ControlFlowNode for FunctionExpr | Function f | builtin-class function | 4 | import | diff --git a/python/ql/test/library-tests/PointsTo/new/PointsToWithType.expected b/python/ql/test/library-tests/PointsTo/new/PointsToWithType.expected index 131957da9fb..31d1a220849 100644 --- a/python/ql/test/library-tests/PointsTo/new/PointsToWithType.expected +++ b/python/ql/test/library-tests/PointsTo/new/PointsToWithType.expected @@ -185,6 +185,8 @@ WARNING: Predicate points_to has been deprecated and may be removed in future (P | b_condition.py:106 | ControlFlowNode for Exception | builtin-class Exception | builtin-class type | 106 | | b_condition.py:106 | ControlFlowNode for Exception() | Exception() | builtin-class Exception | 106 | | b_condition.py:107 | ControlFlowNode for Str | 'Hello' | builtin-class str | 107 | +| b_condition.py:109 | ControlFlowNode for FunctionExpr | Function method_check | builtin-class function | 109 | +| b_condition.py:109 | ControlFlowNode for method_check | Function method_check | builtin-class function | 109 | | d_globals.py:2 | ControlFlowNode for FunctionExpr | Function j | builtin-class function | 2 | | d_globals.py:2 | ControlFlowNode for j | Function j | builtin-class function | 2 | | d_globals.py:3 | ControlFlowNode for Tuple | Tuple | builtin-class tuple | 3 | diff --git a/python/ql/test/library-tests/PointsTo/new/SSA.expected b/python/ql/test/library-tests/PointsTo/new/SSA.expected index 5d8c600a433..c42c07241b2 100644 --- a/python/ql/test/library-tests/PointsTo/new/SSA.expected +++ b/python/ql/test/library-tests/PointsTo/new/SSA.expected @@ -86,6 +86,7 @@ WARNING: Predicate ssa_variable_points_to has been deprecated and may be removed | b_condition.py:96 | y_6 = SingleSuccessorGuard(y_5) [false] | NoneType None | builtin-class NoneType | | b_condition.py:97 | x_3 = ArgumentRefinement(x_2) | NoneType None | builtin-class NoneType | | b_condition.py:101 | not_or_not_1 = FunctionExpr | Function not_or_not | builtin-class function | +| b_condition.py:109 | method_check_1 = FunctionExpr | Function method_check | builtin-class function | | c_tests.py:0 | __name___0 = ScopeEntryDefinition | 'code.c_tests' | builtin-class str | | c_tests.py:4 | f_0 = FunctionExpr | Function f | builtin-class function | | c_tests.py:5 | x_0 = IfExp | NoneType None | builtin-class NoneType | diff --git a/python/ql/test/library-tests/PointsTo/new/SourceEdgeDefinitions.expected b/python/ql/test/library-tests/PointsTo/new/SourceEdgeDefinitions.expected index 900b5ee9152..40c4d753fb2 100644 --- a/python/ql/test/library-tests/PointsTo/new/SourceEdgeDefinitions.expected +++ b/python/ql/test/library-tests/PointsTo/new/SourceEdgeDefinitions.expected @@ -20,3 +20,4 @@ | b_condition.py:102 | Local Variable a | ControlFlowNode for a | | b_condition.py:104 | Local Variable a | ControlFlowNode for a | | b_condition.py:105 | Local Variable a | ControlFlowNode for a | +| b_condition.py:110 | Local Variable x | ControlFlowNode for x | diff --git a/python/ql/test/library-tests/PointsTo/new/SourceNodeDefinitions.expected b/python/ql/test/library-tests/PointsTo/new/SourceNodeDefinitions.expected index e3cffec2918..a36aeeb3afb 100644 --- a/python/ql/test/library-tests/PointsTo/new/SourceNodeDefinitions.expected +++ b/python/ql/test/library-tests/PointsTo/new/SourceNodeDefinitions.expected @@ -50,6 +50,7 @@ | b_condition.py:0 | Global Variable h | Entry node for Module code.b_condition | definition | | b_condition.py:0 | Global Variable k | Entry node for Module code.b_condition | definition | | b_condition.py:0 | Global Variable loop | Entry node for Module code.b_condition | definition | +| b_condition.py:0 | Global Variable method_check | Entry node for Module code.b_condition | definition | | b_condition.py:0 | Global Variable not_or_not | Entry node for Module code.b_condition | definition | | b_condition.py:0 | Global Variable odasa6261 | Entry node for Module code.b_condition | definition | | b_condition.py:0 | Global Variable split_bool1 | Entry node for Module code.b_condition | definition | @@ -115,3 +116,7 @@ | b_condition.py:101 | Global Variable not_or_not | ControlFlowNode for not_or_not | definition | | b_condition.py:101 | Local Variable a | ControlFlowNode for a | definition | | b_condition.py:101 | Local Variable a | Entry node for Function not_or_not | definition | +| b_condition.py:109 | Global Variable method_check | ControlFlowNode for method_check | definition | +| b_condition.py:109 | Local Variable x | ControlFlowNode for x | definition | +| b_condition.py:111 | Local Variable x | ControlFlowNode for use() | refinement | +| b_condition.py:113 | Local Variable x | ControlFlowNode for use() | refinement | diff --git a/python/ql/test/library-tests/PointsTo/new/SsaUses.expected b/python/ql/test/library-tests/PointsTo/new/SsaUses.expected index 04a81665b68..4733d44dcf5 100644 --- a/python/ql/test/library-tests/PointsTo/new/SsaUses.expected +++ b/python/ql/test/library-tests/PointsTo/new/SsaUses.expected @@ -63,6 +63,7 @@ | b_condition.py:0 | h_1 | Exit node for Module code.b_condition | | b_condition.py:0 | k_1 | Exit node for Module code.b_condition | | b_condition.py:0 | loop_1 | Exit node for Module code.b_condition | +| b_condition.py:0 | method_check_1 | Exit node for Module code.b_condition | | b_condition.py:0 | not_or_not_1 | Exit node for Module code.b_condition | | b_condition.py:0 | odasa6261_1 | Exit node for Module code.b_condition | | b_condition.py:0 | split_bool1_1 | Exit node for Module code.b_condition | @@ -91,6 +92,7 @@ | b_condition.py:39 | h_0 | ControlFlowNode for thing() | | b_condition.py:39 | k_0 | ControlFlowNode for thing() | | b_condition.py:39 | loop_0 | ControlFlowNode for thing() | +| b_condition.py:39 | method_check_0 | ControlFlowNode for thing() | | b_condition.py:39 | not_or_not_0 | ControlFlowNode for thing() | | b_condition.py:39 | odasa6261_0 | ControlFlowNode for thing() | | b_condition.py:39 | split_bool1_0 | ControlFlowNode for thing() | @@ -105,6 +107,7 @@ | b_condition.py:43 | h_0 | ControlFlowNode for use() | | b_condition.py:43 | k_0 | ControlFlowNode for use() | | b_condition.py:43 | loop_0 | ControlFlowNode for use() | +| b_condition.py:43 | method_check_0 | ControlFlowNode for use() | | b_condition.py:43 | not_or_not_0 | ControlFlowNode for use() | | b_condition.py:43 | odasa6261_0 | ControlFlowNode for use() | | b_condition.py:43 | split_bool1_0 | ControlFlowNode for use() | @@ -118,6 +121,7 @@ | b_condition.py:44 | h_0 | ControlFlowNode for use() | | b_condition.py:44 | k_0 | ControlFlowNode for use() | | b_condition.py:44 | loop_0 | ControlFlowNode for use() | +| b_condition.py:44 | method_check_0 | ControlFlowNode for use() | | b_condition.py:44 | not_or_not_0 | ControlFlowNode for use() | | b_condition.py:44 | odasa6261_0 | ControlFlowNode for use() | | b_condition.py:44 | split_bool1_0 | ControlFlowNode for use() | @@ -168,6 +172,10 @@ | b_condition.py:102 | a_0 | ControlFlowNode for a | | b_condition.py:104 | a_1 | ControlFlowNode for a | | b_condition.py:105 | a_2 | ControlFlowNode for a | +| b_condition.py:109 | x_5 | Exit node for Function method_check | +| b_condition.py:110 | x_0 | ControlFlowNode for x | +| b_condition.py:111 | x_1 | ControlFlowNode for x | +| b_condition.py:113 | x_3 | ControlFlowNode for x | | d_globals.py:0 | D_1 | Exit node for Module code.d_globals | | d_globals.py:0 | Ugly_1 | Exit node for Module code.d_globals | | d_globals.py:0 | X_1 | Exit node for Module code.d_globals | diff --git a/python/ql/test/library-tests/PointsTo/new/Values.expected b/python/ql/test/library-tests/PointsTo/new/Values.expected index d89f6a4fdef..db8da913c00 100644 --- a/python/ql/test/library-tests/PointsTo/new/Values.expected +++ b/python/ql/test/library-tests/PointsTo/new/Values.expected @@ -139,6 +139,7 @@ | b_condition.py:106 | ControlFlowNode for Exception | runtime | builtin-class Exception | builtin-class type | | b_condition.py:106 | ControlFlowNode for Exception() | runtime | Exception() | builtin-class Exception | | b_condition.py:107 | ControlFlowNode for Str | runtime | 'Hello' | builtin-class str | +| b_condition.py:109 | ControlFlowNode for FunctionExpr | import | Function method_check | builtin-class function | | e_temporal.py:2 | ControlFlowNode for ImportExpr | import | Module sys | builtin-class module | | e_temporal.py:4 | ControlFlowNode for FunctionExpr | import | Function f | builtin-class function | | e_temporal.py:5 | ControlFlowNode for Attribute | code/e_temporal.py:12 from import | list object | builtin-class list | diff --git a/python/ql/test/library-tests/PointsTo/new/VarUses.expected b/python/ql/test/library-tests/PointsTo/new/VarUses.expected index 18fb97ea47a..83bf756373e 100644 --- a/python/ql/test/library-tests/PointsTo/new/VarUses.expected +++ b/python/ql/test/library-tests/PointsTo/new/VarUses.expected @@ -55,6 +55,7 @@ | b_condition.py:0 | k | Exit node for Module code.b_condition | | b_condition.py:0 | list | Exit node for Module code.b_condition | | b_condition.py:0 | loop | Exit node for Module code.b_condition | +| b_condition.py:0 | method_check | Exit node for Module code.b_condition | | b_condition.py:0 | not_or_not | Exit node for Module code.b_condition | | b_condition.py:0 | object | Exit node for Module code.b_condition | | b_condition.py:0 | odasa6261 | Exit node for Module code.b_condition | @@ -109,6 +110,7 @@ | b_condition.py:39 | h | ControlFlowNode for thing() | | b_condition.py:39 | k | ControlFlowNode for thing() | | b_condition.py:39 | loop | ControlFlowNode for thing() | +| b_condition.py:39 | method_check | ControlFlowNode for thing() | | b_condition.py:39 | not_or_not | ControlFlowNode for thing() | | b_condition.py:39 | odasa6261 | ControlFlowNode for thing() | | b_condition.py:39 | split_bool1 | ControlFlowNode for thing() | @@ -124,6 +126,7 @@ | b_condition.py:43 | h | ControlFlowNode for use() | | b_condition.py:43 | k | ControlFlowNode for use() | | b_condition.py:43 | loop | ControlFlowNode for use() | +| b_condition.py:43 | method_check | ControlFlowNode for use() | | b_condition.py:43 | not_or_not | ControlFlowNode for use() | | b_condition.py:43 | odasa6261 | ControlFlowNode for use() | | b_condition.py:43 | split_bool1 | ControlFlowNode for use() | @@ -138,6 +141,7 @@ | b_condition.py:44 | h | ControlFlowNode for use() | | b_condition.py:44 | k | ControlFlowNode for use() | | b_condition.py:44 | loop | ControlFlowNode for use() | +| b_condition.py:44 | method_check | ControlFlowNode for use() | | b_condition.py:44 | not_or_not | ControlFlowNode for use() | | b_condition.py:44 | odasa6261 | ControlFlowNode for use() | | b_condition.py:44 | split_bool1 | ControlFlowNode for use() | @@ -202,6 +206,12 @@ | b_condition.py:104 | a | ControlFlowNode for a | | b_condition.py:105 | a | ControlFlowNode for a | | b_condition.py:106 | Exception | ControlFlowNode for Exception | +| b_condition.py:109 | x | Exit node for Function method_check | +| b_condition.py:110 | x | ControlFlowNode for x | +| b_condition.py:111 | use | ControlFlowNode for use | +| b_condition.py:111 | x | ControlFlowNode for x | +| b_condition.py:113 | use | ControlFlowNode for use | +| b_condition.py:113 | x | ControlFlowNode for x | | d_globals.py:0 | D | Exit node for Module code.d_globals | | d_globals.py:0 | Ugly | Exit node for Module code.d_globals | | d_globals.py:0 | X | Exit node for Module code.d_globals | diff --git a/python/ql/test/library-tests/PointsTo/new/code/b_condition.py b/python/ql/test/library-tests/PointsTo/new/code/b_condition.py index 7574955ca96..d2d5e78d598 100644 --- a/python/ql/test/library-tests/PointsTo/new/code/b_condition.py +++ b/python/ql/test/library-tests/PointsTo/new/code/b_condition.py @@ -106,3 +106,10 @@ def not_or_not(*a): raise Exception() "Hello" +def method_check(x): + if x.m(): + use(x) + else: + use(x) + + From e66c132bae3f4d58cb06316008b9a5174c9fc1ee Mon Sep 17 00:00:00 2001 From: alexet Date: Mon, 10 Jun 2019 11:56:07 +0100 Subject: [PATCH 3/9] Python: Improve performance of submodule name computation. --- python/ql/src/semmle/python/objects/Modules.qll | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/python/ql/src/semmle/python/objects/Modules.qll b/python/ql/src/semmle/python/objects/Modules.qll index c71bd1112b3..531d5527966 100644 --- a/python/ql/src/semmle/python/objects/Modules.qll +++ b/python/ql/src/semmle/python/objects/Modules.qll @@ -155,7 +155,12 @@ class PackageObjectInternal extends ModuleObjectInternal, TPackageObject { /** Gets the submodule `name` of this package */ ModuleObjectInternal submodule(string name) { - result.getName() = this.getName() + "." + name + exists(string fullName, int lastDotIndex | + fullName = result.getName() and + lastDotIndex = max(fullName.indexOf(".")) and + name = fullName.substring(lastDotIndex + 1, fullName.length()) and + this.getName() = fullName.substring(0, lastDotIndex) + ) } override int intValue() { From f04bc2668403cff716a247e50b692b7f52a9cf19 Mon Sep 17 00:00:00 2001 From: alexet Date: Mon, 10 Jun 2019 17:07:28 +0100 Subject: [PATCH 4/9] Python: Improve points-to performance on large databases. --- python/ql/src/semmle/python/pointsto/PointsTo.qll | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/python/ql/src/semmle/python/pointsto/PointsTo.qll b/python/ql/src/semmle/python/pointsto/PointsTo.qll index 771ada01956..9416dbd6cd8 100644 --- a/python/ql/src/semmle/python/pointsto/PointsTo.qll +++ b/python/ql/src/semmle/python/pointsto/PointsTo.qll @@ -170,6 +170,11 @@ module PointsTo { cached module PointsToInternal { + pragma[noinline] + cached predicate importCtxPointsTo(ControlFlowNode f, ObjectInternal value, ControlFlowNode origin) { + PointsToInternal::pointsTo(f, any(Context ctx | ctx.isImport()), value, origin) + } + /** INTERNAL -- Use `f.refersTo(value, origin)` instead. */ cached predicate pointsTo(ControlFlowNode f, PointsToContext context, ObjectInternal value, ControlFlowNode origin) { points_to_candidate(f, context, value, origin) and @@ -695,7 +700,7 @@ private module InterModulePointsTo { predicate ofInterestInExports(ModuleObjectInternal mod, string name) { exists(ImportStarNode imp, ImportStarRefinement def, EssaVariable var | imp = def.getDefiningNode() and - PointsToInternal::pointsTo(imp.getModule(), any(Context ctx | ctx.isImport()), mod, _) and + PointsToInternal::importCtxPointsTo(imp.getModule(), mod, _) and var = def.getVariable() | if var.isMetaVariable() then ( @@ -878,7 +883,7 @@ module InterProceduralPointsTo { /** Helper for parameter_points_to */ pragma [noinline] private predicate default_parameter_points_to(ParameterDefinition def, PointsToContext context, ObjectInternal value, ControlFlowNode origin) { - exists(PointsToContext imp | imp.isImport() | PointsToInternal::pointsTo(def.getDefault(), imp, value, origin)) and + PointsToInternal::importCtxPointsTo(def.getDefault(), value, origin) and context_for_default_value(def, context) } @@ -2198,7 +2203,7 @@ cached module ModuleAttributes { private predicate importStarDef(ImportStarRefinement def, EssaVariable input, ModuleObjectInternal mod) { exists(ImportStarNode imp | def.getVariable().getName() = "$" and imp = def.getDefiningNode() and - input = def.getInput() and PointsToInternal::pointsTo(imp.getModule(), any(Context ctx | ctx.isImport()), mod, _) + input = def.getInput() and PointsToInternal::importCtxPointsTo(imp.getModule(), mod, _) ) } From 0fb323b5ff83933776b33ea31b07ec33e6844dcc Mon Sep 17 00:00:00 2001 From: yh-semmle Date: Mon, 10 Jun 2019 12:59:47 -0400 Subject: [PATCH 5/9] Java: add QL library for modeling `AndroidManifest.xml` files --- .../src/semmle/code/xml/AndroidManifest.qll | 178 ++++++++++++++++++ .../android/manifest/AndroidManifest.xml | 48 +++++ .../frameworks/android/manifest/Test.java | 1 + .../android/manifest/manifest.expected | 1 + .../frameworks/android/manifest/manifest.ql | 5 + 5 files changed, 233 insertions(+) create mode 100644 java/ql/src/semmle/code/xml/AndroidManifest.qll create mode 100644 java/ql/test/library-tests/frameworks/android/manifest/AndroidManifest.xml create mode 100644 java/ql/test/library-tests/frameworks/android/manifest/Test.java create mode 100644 java/ql/test/library-tests/frameworks/android/manifest/manifest.expected create mode 100644 java/ql/test/library-tests/frameworks/android/manifest/manifest.ql diff --git a/java/ql/src/semmle/code/xml/AndroidManifest.qll b/java/ql/src/semmle/code/xml/AndroidManifest.qll new file mode 100644 index 00000000000..366b2306a46 --- /dev/null +++ b/java/ql/src/semmle/code/xml/AndroidManifest.qll @@ -0,0 +1,178 @@ +/** + * Provides classes and predicates for working with Android manifest files. + */ + +import XML + +/** + * An Android manifest file, named `AndroidManifest.xml`. + */ +class AndroidManifestXmlFile extends XMLFile { + AndroidManifestXmlFile() { + this.getBaseName() = "AndroidManifest.xml" and + count(XMLElement e | e = this.getAChild()) = 1 and + this.getAChild().getName() = "manifest" + } + + /** + * Gets the top-level `` element in this Android manifest file. + */ + AndroidManifestXmlElement getManifestElement() { result = this.getAChild() } +} + +/** + * A `` element in an Android manifest file. + */ +class AndroidManifestXmlElement extends XMLElement { + AndroidManifestXmlElement() { + this.getParent() instanceof AndroidManifestXmlFile and this.getName() = "manifest" + } + + /** + * Gets the `` child element of this `` element. + */ + AndroidApplicationXmlElement getApplicationElement() { result = this.getAChild() } + + /** + * Gets the value of the `package` attribute of this `` element. + */ + string getPackageAttributeValue() { result = getAttributeValue("package") } +} + +/** + * An `` element in an Android manifest file. + */ +class AndroidApplicationXmlElement extends XMLElement { + AndroidApplicationXmlElement() { + this.getParent() instanceof AndroidManifestXmlElement and this.getName() = "application" + } + + /** + * Gets a component child element of this `` element. + */ + AndroidComponentXmlElement getAComponentElement() { result = this.getAChild() } +} + +/** + * An `` element in an Android manifest file. + */ +class AndroidActivityXmlElement extends AndroidComponentXmlElement { + AndroidActivityXmlElement() { this.getName() = "activity" } +} + +/** + * A `` element in an Android manifest file. + */ +class AndroidServiceXmlElement extends AndroidComponentXmlElement { + AndroidServiceXmlElement() { this.getName() = "service" } +} + +/** + * A `` element in an Android manifest file. + */ +class AndroidReceiverXmlElement extends AndroidComponentXmlElement { + AndroidReceiverXmlElement() { this.getName() = "receiver" } +} + +/** + * A `` element in an Android manifest file. + */ +class AndroidProviderXmlElement extends AndroidComponentXmlElement { + AndroidProviderXmlElement() { this.getName() = "provider" } +} + +/** + * An Android component element in an Android manifest file. + */ +class AndroidComponentXmlElement extends XMLElement { + AndroidComponentXmlElement() { + this.getParent() instanceof AndroidApplicationXmlElement and + this.getName().regexpMatch("(activity|service|receiver|provider)") + } + + /** + * Gets an `` child element of this component element. + */ + AndroidIntentFilterXmlElement getAnIntentFilterElement() { result = this.getAChild() } + + /** + * Gets the value of the `android:name` attribute of this component element. + */ + string getComponentName() { + exists(XMLAttribute attr | + attr = getAnAttribute() and + attr.getNamespace().getPrefix() = "android" and + attr.getName() = "name" + | + result = attr.getValue() + ) + } + + /** + * Gets the resolved value of the `android:name` attribute of this component element. + */ + string getResolvedComponentName() { + if getComponentName().matches(".%") + then + result = getParent() + .(XMLElement) + .getParent() + .(AndroidManifestXmlElement) + .getPackageAttributeValue() + getComponentName() + else result = getComponentName() + } + + /** + * Gets the value of the `android:exported` attribute of this component element. + */ + string getExportedAttributeValue() { + exists(XMLAttribute attr | + attr = getAnAttribute() and + attr.getNamespace().getPrefix() = "android" and + attr.getName() = "exported" + | + result = attr.getValue() + ) + } + + /** + * Holds if the `android:exported` attribute of this component element is `true`. + */ + predicate isExported() { getExportedAttributeValue() = "true" } +} + +/** + * An `` element in an Android manifest file. + */ +class AndroidIntentFilterXmlElement extends XMLElement { + AndroidIntentFilterXmlElement() { + this.getFile() instanceof AndroidManifestXmlFile and this.getName() = "intent-filter" + } + + /** + * Gets an `` child element of this `` element. + */ + AndroidActionXmlElement getAnActionElement() { result = this.getAChild() } +} + +/** + * An `` element in an Android manifest file. + */ +class AndroidActionXmlElement extends XMLElement { + AndroidActionXmlElement() { + this.getFile() instanceof AndroidManifestXmlFile and this.getName() = "action" + } + + /** + * Gets the name of this action. + */ + string getActionName() { + exists(XMLAttribute attr | + attr = getAnAttribute() and + attr.getNamespace().getPrefix() = "android" and + attr.getName() = "name" + | + result = attr.getValue() + ) + } +} diff --git a/java/ql/test/library-tests/frameworks/android/manifest/AndroidManifest.xml b/java/ql/test/library-tests/frameworks/android/manifest/AndroidManifest.xml new file mode 100644 index 00000000000..2a1160f8841 --- /dev/null +++ b/java/ql/test/library-tests/frameworks/android/manifest/AndroidManifest.xml @@ -0,0 +1,48 @@ + + + + + + + + + + + + + + + + + + + + + diff --git a/java/ql/test/library-tests/frameworks/android/manifest/Test.java b/java/ql/test/library-tests/frameworks/android/manifest/Test.java new file mode 100644 index 00000000000..ef22a69f193 --- /dev/null +++ b/java/ql/test/library-tests/frameworks/android/manifest/Test.java @@ -0,0 +1 @@ +class Test {} diff --git a/java/ql/test/library-tests/frameworks/android/manifest/manifest.expected b/java/ql/test/library-tests/frameworks/android/manifest/manifest.expected new file mode 100644 index 00000000000..203a223f2f0 --- /dev/null +++ b/java/ql/test/library-tests/frameworks/android/manifest/manifest.expected @@ -0,0 +1 @@ +| com.example.myapp.MainActivity | android.intent.action.MAIN | diff --git a/java/ql/test/library-tests/frameworks/android/manifest/manifest.ql b/java/ql/test/library-tests/frameworks/android/manifest/manifest.ql new file mode 100644 index 00000000000..7311aa1fdab --- /dev/null +++ b/java/ql/test/library-tests/frameworks/android/manifest/manifest.ql @@ -0,0 +1,5 @@ +import java +import semmle.code.xml.AndroidManifest + +from AndroidActivityXmlElement e +select e.getResolvedComponentName(), e.getAnIntentFilterElement().getAnActionElement().getActionName() From 8e6b62a301a0d04190b21bca513e5826e68eab16 Mon Sep 17 00:00:00 2001 From: yh-semmle Date: Mon, 10 Jun 2019 13:00:03 -0400 Subject: [PATCH 6/9] Java: add QL library for modeling Android components --- .../code/java/frameworks/android/Android.qll | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 java/ql/src/semmle/code/java/frameworks/android/Android.qll diff --git a/java/ql/src/semmle/code/java/frameworks/android/Android.qll b/java/ql/src/semmle/code/java/frameworks/android/Android.qll new file mode 100644 index 00000000000..a16c43ddfc5 --- /dev/null +++ b/java/ql/src/semmle/code/java/frameworks/android/Android.qll @@ -0,0 +1,54 @@ +/** + * Provides classes and predicates for working with Android components. + */ + +import java +import semmle.code.xml.AndroidManifest + +/** + * An Android component. That is, either an activity, a service, + * a broadcast receiver, or a content provider. + */ +class AndroidComponent extends Class { + AndroidComponent() { + this.getASupertype*().hasQualifiedName("android.app", "Activity") or + this.getASupertype*().hasQualifiedName("android.app", "Service") or + this.getASupertype*().hasQualifiedName("android.content", "BroadcastReceiver") or + this.getASupertype*().hasQualifiedName("android.content", "ContentProvider") + } + + /** The XML element corresponding to this Android component. */ + AndroidComponentXmlElement getAndroidComponentXmlElement() { + result.getResolvedComponentName() = this.getQualifiedName() + } + + /** Holds if this Android component is configured as `exported` in an `AndroidManifest.xml` file. */ + predicate isExported() { getAndroidComponentXmlElement().isExported() } + + /** Holds if this Android component has an intent filter configured in an `AndroidManifest.xml` file. */ + predicate hasIntentFilter() { exists(getAndroidComponentXmlElement().getAnIntentFilterElement()) } +} + +/** An Android activity. */ +class AndroidActivity extends AndroidComponent { + AndroidActivity() { this.getASupertype*().hasQualifiedName("android.app", "Activity") } +} + +/** An Android service. */ +class AndroidService extends AndroidComponent { + AndroidService() { this.getASupertype*().hasQualifiedName("android.app", "Service") } +} + +/** An Android broadcast receiver. */ +class AndroidBroadcastReceiver extends AndroidComponent { + AndroidBroadcastReceiver() { + this.getASupertype*().hasQualifiedName("android.content", "BroadcastReceiver") + } +} + +/** An Android content provider. */ +class AndroidContentProvider extends AndroidComponent { + AndroidContentProvider() { + this.getASupertype*().hasQualifiedName("android.content", "ContentProvider") + } +} From 16b151745bb41f22a075774408645a9ae17e5612 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 11 Jun 2019 09:44:50 +0200 Subject: [PATCH 7/9] C++: use shortestDistances in PrimitiveBasicBlocks The use of transitive closure for BB index calculation has been the cause of an out-of-memory error. This commit switches the calculation to use the `shortestDistances` HOP, which still has the problem that the result needs to fit in RAM, but at least the RAM requirements are sure to be linear in the size of the result. The `shortestDistances` HOP is already used for BB index calculation for the C++ IR and for C#. We could guard even better against OOM by switching the calculation to use manual recursion, but that would undo the much-needed performance improvements we got from #123. This change improves performance on Wireshark, which is notorious for having long basic blocks. When I benchmarked `shortestDistances` for #123, it was slower than TC. With the current evaluator, it looks like `shortestDistances` is faster. Performance before was: PrimitiveBasicBlocks::Cached::getMemberIndex#ff ................... 9.7s (executed 8027 times) #PrimitiveBasicBlocks::Cached::member_step#ffPlus ................. 6.6s PrimitiveBasicBlocks::Cached::primitive_basic_block_entry_node#f .. 3.5s PrimitiveBasicBlocks::Cached::primitive_basic_block_member#fff .... 2.3s Performance with this commit is: PrimitiveBasicBlocks::Cached::primitive_basic_block_entry_node#f ................................................................... 3.5s shortestDistances@PrimitiveBasicBlocks::Cached::primitive_basic_block_entry_node#1@PrimitiveBasicBlocks::Cached::member_step#2#fff . 3s PrimitiveBasicBlocks::Cached::primitive_basic_block_member#fff ..................................................................... 963ms --- .../internal/PrimitiveBasicBlocks.qll | 23 ++----------------- 1 file changed, 2 insertions(+), 21 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll b/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll index 27538b40592..e7b2e1271ff 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll @@ -60,29 +60,10 @@ private cached module Cached { not n2 instanceof PrimitiveBasicBlock } - /** Returns the index of `node` in its `PrimitiveBasicBlock`. */ - private int getMemberIndex(Node node) { - primitive_basic_block_entry_node(node) and - result = 0 - or - exists(Node prev | - member_step(prev, node) and - result = getMemberIndex(prev) + 1 - ) - } - /** Holds if `node` is the `pos`th control-flow node in primitive basic block `bb`. */ cached - predicate primitive_basic_block_member(Node node, PrimitiveBasicBlock bb, int pos) { - primitive_basic_block_entry_node(bb) and - ( - pos = 0 and - node = bb - or - pos = getMemberIndex(node) and - member_step+(bb, node) - ) - } + predicate primitive_basic_block_member(Node node, PrimitiveBasicBlock bb, int pos) = + shortestDistances(primitive_basic_block_entry_node/1, member_step/2)(bb, node, pos) /** Gets the number of control-flow nodes in the primitive basic block `bb`. */ cached From 32122e86b04bddc4628d7a1be72e09e4603e0008 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Tue, 11 Jun 2019 14:30:21 +0200 Subject: [PATCH 8/9] C++: use plain recursion in PrimitiveBasicBlocks It's sometimes faster but sometimes up to 2x slower to use plain recursion here. On the other hand, plain recursion won't run out of Java heap space, and it won't make unrelated computation slower by forcing all RAM data out to disk. --- .../cpp/controlflow/internal/PrimitiveBasicBlocks.qll | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll b/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll index e7b2e1271ff..f9f82c53349 100644 --- a/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll +++ b/cpp/ql/src/semmle/code/cpp/controlflow/internal/PrimitiveBasicBlocks.qll @@ -62,8 +62,14 @@ private cached module Cached { /** Holds if `node` is the `pos`th control-flow node in primitive basic block `bb`. */ cached - predicate primitive_basic_block_member(Node node, PrimitiveBasicBlock bb, int pos) = - shortestDistances(primitive_basic_block_entry_node/1, member_step/2)(bb, node, pos) + predicate primitive_basic_block_member(Node node, PrimitiveBasicBlock bb, int pos) { + primitive_basic_block_entry_node(bb) and node = bb and pos = 0 + or + exists(Node prev | + member_step(prev, node) and + primitive_basic_block_member(prev, bb, pos - 1) + ) + } /** Gets the number of control-flow nodes in the primitive basic block `bb`. */ cached From 0c02d3deefa77d285de6994b5e6e2f0fc77f77a8 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Tue, 11 Jun 2019 15:42:41 +0100 Subject: [PATCH 9/9] Python: Fix up expected test results for six test. --- python/ql/test/3/library-tests/six/test.expected | 9 --------- 1 file changed, 9 deletions(-) diff --git a/python/ql/test/3/library-tests/six/test.expected b/python/ql/test/3/library-tests/six/test.expected index 2e52b001840..199ab676f4f 100644 --- a/python/ql/test/3/library-tests/six/test.expected +++ b/python/ql/test/3/library-tests/six/test.expected @@ -46,14 +46,6 @@ | Module six | iterlists | Function iterlists | | Module six | itervalues | Function itervalues | | Module six | moves | Module six.moves | -| Module six | moves.__init__ | Module six.moves.__init__ | -| Module six | moves.urllib | Module six.moves.urllib | -| Module six | moves.urllib.__init__ | Module six.moves.urllib.__init__ | -| Module six | moves.urllib_error | Module six.moves.urllib_error | -| Module six | moves.urllib_parse | Module six.moves.urllib_parse | -| Module six | moves.urllib_request | Module six.moves.urllib_request | -| Module six | moves.urllib_response | Module six.moves.urllib_response | -| Module six | moves.urllib_robotparser | Module six.moves.urllib_robotparser | | Module six | next | Builtin-function next | | Module six | operator | Module operator | | Module six | print_ | Function print_ | @@ -174,7 +166,6 @@ | Module six.moves | tkinter_tksimpledialog | Module tkinter.simpledialog | | Module six.moves | tkinter_ttk | Module tkinter.ttk | | Module six.moves | urllib | Module six.moves.urllib | -| Module six.moves | urllib.__init__ | Module six.moves.urllib.__init__ | | Module six.moves | urllib_error | Module six.moves.urllib_error | | Module six.moves | urllib_parse | Module six.moves.urllib_parse | | Module six.moves | urllib_request | Module six.moves.urllib_request |