Skip to content

Commit a877311

Browse files
authored
Merge pull request github#5860 from max-schaefer/js/improve-sql-modelling
Approved by asgerf
2 parents beb66fc + 8f91e9e commit a877311

File tree

4 files changed

+18
-3
lines changed

4 files changed

+18
-3
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
lgtm,codescanning
2+
* Modelling of chaining methods in the `sqlite3` package has improved, which may lead to
3+
additional results from the `js/sql-injection` query.

javascript/ql/src/semmle/javascript/frameworks/SQL.qll

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,18 +341,28 @@ private module Sqlite {
341341
result = sqlite().getMember("verbose").getReturn()
342342
}
343343

344-
/** Gets an expression that constructs a Sqlite database instance. */
344+
/** Gets an expression that constructs or returns a Sqlite database instance. */
345345
API::Node database() {
346346
// new require('sqlite3').Database()
347347
result = sqlite().getMember("Database").getInstance()
348348
or
349+
// chained call
350+
result = getAChainingQueryCall()
351+
or
349352
result = API::Node::ofType("sqlite3", "Database")
350353
}
351354

355+
/** A call to a query method on a Sqlite database instance that returns the same instance. */
356+
private API::Node getAChainingQueryCall() {
357+
result = database().getMember(["all", "each", "exec", "get", "run"]).getReturn()
358+
}
359+
352360
/** A call to a Sqlite query method. */
353361
private class QueryCall extends DatabaseAccess, DataFlow::MethodCallNode {
354362
QueryCall() {
355-
this = database().getMember(["all", "each", "exec", "get", "prepare", "run"]).getACall()
363+
this = getAChainingQueryCall().getAnImmediateUse()
364+
or
365+
this = database().getMember("prepare").getACall()
356366
}
357367

358368
override DataFlow::Node getAQueryArgument() { result = getArgument(0) }

javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,5 +66,6 @@
6666
| spannerImport.js:4:8:4:17 | "SQL code" |
6767
| sqlite-types.ts:4:12:4:49 | "UPDATE ... id = ?" |
6868
| sqlite.js:7:8:7:45 | "UPDATE ... id = ?" |
69+
| sqlite.js:8:8:8:45 | "UPDATE ... id = ?" |
6970
| sqliteArray.js:6:12:6:49 | "UPDATE ... id = ?" |
7071
| sqliteImport.js:2:8:2:44 | "UPDATE ... id = ?" |

javascript/ql/test/library-tests/frameworks/SQL/sqlite.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
var sqlite = require('sqlite3');
55

66
var db = new sqlite.Database(":memory:");
7-
db.run("UPDATE tbl SET name = ? WHERE id = ?", "bar", 2);
7+
db.run("UPDATE tbl SET name = ? WHERE id = ?", "bar", 2)
8+
.run("UPDATE tbl SET name = ? WHERE id = ?", "foo", 3);
89

910
exports.db = db;

0 commit comments

Comments
 (0)