diff --git a/session/session.ql b/session/session.ql index 9a5d4a8..04833bf 100644 --- a/session/session.ql +++ b/session/session.ql @@ -32,10 +32,6 @@ class FlowSinkOrigin extends DataFlow::FlowLabel { FlowSinkOrigin() { this = "FlowSinkOrigin" } } -class UltimateFlow extends DataFlow::FlowLabel { - UltimateFlow() { this = "UltimateFlow" } -} - class IdentifyFlowSink extends TaintTracking::Configuration { IdentifyFlowSink() { this = "IdentifyFlowSink" } @@ -46,9 +42,6 @@ class IdentifyFlowSink extends TaintTracking::Configuration { nd.asExpr() = newdb and lbl instanceof FlowSinkOrigin ) - or - nd.asExpr() = uSource(_) and - lbl instanceof UltimateFlow } override predicate isSink(DataFlow::Node nd, DataFlow::FlowLabel lbl) { @@ -59,9 +52,6 @@ class IdentifyFlowSink extends TaintTracking::Configuration { nd.asExpr() = db and lbl instanceof FlowSinkOrigin ) - or - nd.asExpr() = uSink(_) and - lbl instanceof UltimateFlow } } @@ -73,9 +63,14 @@ class UltimateFlowCfg extends TaintTracking::Configuration { override predicate isSink(DataFlow::Node nd) { nd.asExpr() = uSink(_) } } -// from IdentifyFlowSink cfg, DataFlow::PathNode source, DataFlow::PathNode sink -// where cfg.hasFlowPath(source, sink) -// select sink, source, sink, "Database originating $@", source, "here" -from UltimateFlowCfg cfg, DataFlow::PathNode source, DataFlow::PathNode sink -where cfg.hasFlowPath(source, sink) -select sink, source, sink, "Sql injected from $@", source, "here" +from + UltimateFlowCfg ucfg, DataFlow::PathNode usource, DataFlow::PathNode usink, IdentifyFlowSink cfg, + DataFlow::Node source, DataFlow::Node sink +where + cfg.hasFlow(source, sink) and + ucfg.hasFlowPath(usource, usink) and + exists(MethodCallExpr exec | + sink.asExpr() = exec.getReceiver() and + usink.getNode().asExpr() = exec.getAnArgument() + ) +select usink, usource, usink, "Sql injected from $@", usource, "here" diff --git a/solutions/RestrictedSqlInjection.ql b/solutions/RestrictedSqlInjection.ql new file mode 100644 index 0000000..04833bf --- /dev/null +++ b/solutions/RestrictedSqlInjection.ql @@ -0,0 +1,76 @@ +/** + * @kind path-problem + */ + +import javascript +import DataFlow::PathGraph + +// Ultimate source +// ---------------- +// var line = stdinBuffer.toString(); +Expr uSource(MethodCallExpr sbts) { + sbts.getMethodName().matches("%toString%") and + result = sbts +} + +// Ultimate sink +// ---------------- +// db.exec(query); +Expr uSink(MethodCallExpr exec) { + exec.getMethodName() = "exec" and + result = exec.getArgument(0) +} + +// Flow sink origin +// ------------------------ +// Connect +// const db = new sqlite3.Database( +// to its use +// db.exec(query); +// +class FlowSinkOrigin extends DataFlow::FlowLabel { + FlowSinkOrigin() { this = "FlowSinkOrigin" } +} + +class IdentifyFlowSink extends TaintTracking::Configuration { + IdentifyFlowSink() { this = "IdentifyFlowSink" } + + override predicate isSource(DataFlow::Node nd, DataFlow::FlowLabel lbl) { + // const db = new sqlite3.Database( + exists(NewExpr newdb | + newdb.getCalleeName() = "Database" and + nd.asExpr() = newdb and + lbl instanceof FlowSinkOrigin + ) + } + + override predicate isSink(DataFlow::Node nd, DataFlow::FlowLabel lbl) { + // db.exec(query); + exists(Expr db, MethodCallExpr exec | + exec.getMethodName() = "exec" and + db = exec.getReceiver() and + nd.asExpr() = db and + lbl instanceof FlowSinkOrigin + ) + } +} + +class UltimateFlowCfg extends TaintTracking::Configuration { + UltimateFlowCfg() { this = "UltimateFlowCfg" } + + override predicate isSource(DataFlow::Node nd) { nd.asExpr() = uSource(_) } + + override predicate isSink(DataFlow::Node nd) { nd.asExpr() = uSink(_) } +} + +from + UltimateFlowCfg ucfg, DataFlow::PathNode usource, DataFlow::PathNode usink, IdentifyFlowSink cfg, + DataFlow::Node source, DataFlow::Node sink +where + cfg.hasFlow(source, sink) and + ucfg.hasFlowPath(usource, usink) and + exists(MethodCallExpr exec | + sink.asExpr() = exec.getReceiver() and + usink.getNode().asExpr() = exec.getAnArgument() + ) +select usink, usource, usink, "Sql injected from $@", usource, "here" diff --git a/tests/RestrictedSqlInjection/RestrictedSqlInjection.expected b/tests/RestrictedSqlInjection/RestrictedSqlInjection.expected new file mode 100644 index 0000000..a66de3b --- /dev/null +++ b/tests/RestrictedSqlInjection/RestrictedSqlInjection.expected @@ -0,0 +1,54 @@ +nodes +| add-user.js:4:9:4:37 | line | +| add-user.js:4:16:4:37 | stdinBu ... tring() | +| add-user.js:4:16:4:37 | stdinBu ... tring() | +| add-user.js:6:5:6:45 | line | +| add-user.js:6:12:6:15 | line | +| add-user.js:6:12:6:45 | line.re ... gm, "") | +| add-user.js:7:12:7:15 | line | +| add-user.js:16:11:26:10 | db | +| add-user.js:16:16:26:10 | new sql ... }) | +| add-user.js:16:16:26:10 | new sql ... }) | +| add-user.js:28:12:28:13 | db | +| add-user.js:31:21:31:22 | db | +| add-user.js:31:29:31:32 | info | +| add-user.js:33:11:33:63 | query | +| add-user.js:33:19:33:63 | `INSERT ... nfo}")` | +| add-user.js:33:56:33:59 | info | +| add-user.js:35:5:35:6 | db | +| add-user.js:35:5:35:6 | db | +| add-user.js:35:13:35:17 | query | +| add-user.js:35:13:35:17 | query | +| add-user.js:41:9:41:30 | info | +| add-user.js:41:16:41:30 | get_user_info() | +| add-user.js:43:9:43:25 | db | +| add-user.js:43:14:43:25 | connect_db() | +| add-user.js:44:16:44:17 | db | +| add-user.js:44:24:44:27 | info | +edges +| add-user.js:4:9:4:37 | line | add-user.js:6:12:6:15 | line | +| add-user.js:4:16:4:37 | stdinBu ... tring() | add-user.js:4:9:4:37 | line | +| add-user.js:4:16:4:37 | stdinBu ... tring() | add-user.js:4:9:4:37 | line | +| add-user.js:6:5:6:45 | line | add-user.js:7:12:7:15 | line | +| add-user.js:6:12:6:15 | line | add-user.js:6:12:6:45 | line.re ... gm, "") | +| add-user.js:6:12:6:45 | line.re ... gm, "") | add-user.js:6:5:6:45 | line | +| add-user.js:7:12:7:15 | line | add-user.js:41:16:41:30 | get_user_info() | +| add-user.js:16:11:26:10 | db | add-user.js:28:12:28:13 | db | +| add-user.js:16:16:26:10 | new sql ... }) | add-user.js:16:11:26:10 | db | +| add-user.js:16:16:26:10 | new sql ... }) | add-user.js:16:11:26:10 | db | +| add-user.js:28:12:28:13 | db | add-user.js:43:14:43:25 | connect_db() | +| add-user.js:31:21:31:22 | db | add-user.js:35:5:35:6 | db | +| add-user.js:31:21:31:22 | db | add-user.js:35:5:35:6 | db | +| add-user.js:31:29:31:32 | info | add-user.js:33:56:33:59 | info | +| add-user.js:33:11:33:63 | query | add-user.js:35:13:35:17 | query | +| add-user.js:33:11:33:63 | query | add-user.js:35:13:35:17 | query | +| add-user.js:33:19:33:63 | `INSERT ... nfo}")` | add-user.js:33:11:33:63 | query | +| add-user.js:33:56:33:59 | info | add-user.js:33:19:33:63 | `INSERT ... nfo}")` | +| add-user.js:41:9:41:30 | info | add-user.js:44:24:44:27 | info | +| add-user.js:41:16:41:30 | get_user_info() | add-user.js:41:9:41:30 | info | +| add-user.js:43:9:43:25 | db | add-user.js:44:16:44:17 | db | +| add-user.js:43:14:43:25 | connect_db() | add-user.js:43:9:43:25 | db | +| add-user.js:44:16:44:17 | db | add-user.js:31:21:31:22 | db | +| add-user.js:44:24:44:27 | info | add-user.js:31:29:31:32 | info | +#select +| add-user.js:35:13:35:17 | query | add-user.js:4:16:4:37 | stdinBu ... tring() | add-user.js:35:13:35:17 | query | Sql injected from $@ | add-user.js:4:16:4:37 | stdinBu ... tring() | here | diff --git a/tests/RestrictedSqlInjection/RestrictedSqlInjection.qlref b/tests/RestrictedSqlInjection/RestrictedSqlInjection.qlref new file mode 100644 index 0000000..03d95e9 --- /dev/null +++ b/tests/RestrictedSqlInjection/RestrictedSqlInjection.qlref @@ -0,0 +1 @@ +RestrictedSqlInjection.ql diff --git a/tests/RestrictedSqlInjection/add-user.js b/tests/RestrictedSqlInjection/add-user.js new file mode 100644 index 0000000..a4c8b1c --- /dev/null +++ b/tests/RestrictedSqlInjection/add-user.js @@ -0,0 +1,47 @@ +function get_user_info() { + var fs = require("fs"); + var stdinBuffer = fs.readFileSync(process.stdin.fd); + var line = stdinBuffer.toString(); + console.log(line); + line = line.replace(/(\r\n|\n|\r)/gm, ""); + return line +} + +function get_new_id() { + return Math.floor(Math.random() * 12345); +} + +function connect_db() { + const sqlite3 = require('sqlite3').verbose(); + const db = new sqlite3.Database( + 'users.sqlite', + sqlite3.OPEN_READWRITE | sqlite3.OPEN_FULLMUTEX, + err => { + if (err){ + console.log(err); + throw err; + } else { + console.log('DB opened'); + } + }); + + return db; +} + +function write_info(db, id, info) { + db.serialize(); + const query = `INSERT INTO users VALUES (${id}, "${info}")`; + console.log(query); + db.exec(query); + db.close(); +} + +let add_user = () => { + console.log("Running add-user"); + var info = get_user_info(); + var id = get_new_id(); + var db = connect_db(); + write_info(db, id, info); +} + +add_user()