Skip to content

Commit 9ebbd9e

Browse files
codeql-cicodeql-ci
authored
Merge pull request #7591 from asgerf/js/mysql-sinks
Approved by esbena
2 parents a57ee01 + 79f7990 commit 9ebbd9e

File tree

5 files changed

+49
-1
lines changed

5 files changed

+49
-1
lines changed

‎javascript/ql/lib/semmle/javascript/frameworks/SQL.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,9 @@ private module MySql {
8484

8585
override DataFlow::Node getAResult() { result = this.getCallback(_).getParameter(1) }
8686

87-
override DataFlow::Node getAQueryArgument() { result = this.getArgument(0) }
87+
override DataFlow::Node getAQueryArgument() {
88+
result = this.getArgument(0) or result = this.getOptionArgument(0, "sql")
89+
}
8890
}
8991

9092
/** An expression that is passed to the `query` method and hence interpreted as SQL. */

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
| mssql-types.ts:7:31:7:42 | 'SELECT 123' |
99
| mysql1.js:13:18:13:43 | 'SELECT ... lution' |
1010
| mysql1.js:18:18:22:1 | {\\n s ... vid']\\n} |
11+
| mysql1.js:19:10:19:51 | 'SELECT ... r` = ?' |
1112
| mysql1a.js:17:18:17:43 | 'SELECT ... lution' |
1213
| mysql2-promise.js:14:3:14:62 | 'SELECT ... ` > 45' |
1314
| mysql2-promise.js:23:3:23:56 | 'SELECT ... e` > ?' |
@@ -23,6 +24,7 @@
2324
| mysql3.js:26:14:26:31 | 'SELECT something' |
2425
| mysql4.js:14:18:14:20 | sql |
2526
| mysqlImport.js:3:18:5:1 | {\\n s ... = ?',\\n} |
27+
| mysqlImport.js:4:10:4:51 | 'SELECT ... r` = ?' |
2628
| postgres1.js:37:21:37:24 | text |
2729
| postgres2.js:30:16:30:41 | 'SELECT ... number' |
2830
| postgres2.js:43:15:43:26 | 'SELECT 123' |

‎javascript/ql/test/query-tests/Security/CWE-089/untyped/DatabaseAccesses.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,9 @@
4141
| mongoose.js:97:2:97:52 | Documen ... query)) |
4242
| mongoose.js:99:2:99:50 | Documen ... query)) |
4343
| mongoose.js:113:2:113:53 | Documen ... () { }) |
44+
| mysql.js:8:9:11:47 | connect ... ds) {}) |
45+
| mysql.js:14:9:16:47 | connect ... ds) {}) |
46+
| mysql.js:19:9:20:48 | connect ... ds) {}) |
4447
| pg-promise-types.ts:8:5:8:22 | this.db.one(taint) |
4548
| pg-promise.js:9:3:9:15 | db.any(query) |
4649
| pg-promise.js:10:3:10:16 | db.many(query) |

‎javascript/ql/test/query-tests/Security/CWE-089/untyped/SqlInjection.expected

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,15 @@ nodes
296296
| mongooseModelClient.js:12:22:12:29 | req.body |
297297
| mongooseModelClient.js:12:22:12:29 | req.body |
298298
| mongooseModelClient.js:12:22:12:32 | req.body.id |
299+
| mysql.js:6:9:6:31 | temp |
300+
| mysql.js:6:16:6:31 | req.params.value |
301+
| mysql.js:6:16:6:31 | req.params.value |
302+
| mysql.js:15:18:15:65 | 'SELECT ... + temp |
303+
| mysql.js:15:18:15:65 | 'SELECT ... + temp |
304+
| mysql.js:15:62:15:65 | temp |
305+
| mysql.js:19:26:19:73 | 'SELECT ... + temp |
306+
| mysql.js:19:26:19:73 | 'SELECT ... + temp |
307+
| mysql.js:19:70:19:73 | temp |
299308
| pg-promise-types.ts:7:9:7:28 | taint |
300309
| pg-promise-types.ts:7:17:7:28 | req.params.x |
301310
| pg-promise-types.ts:7:17:7:28 | req.params.x |
@@ -792,6 +801,14 @@ edges
792801
| mongooseModelClient.js:12:22:12:29 | req.body | mongooseModelClient.js:12:22:12:32 | req.body.id |
793802
| mongooseModelClient.js:12:22:12:32 | req.body.id | mongooseModelClient.js:12:16:12:34 | { id: req.body.id } |
794803
| mongooseModelClient.js:12:22:12:32 | req.body.id | mongooseModelClient.js:12:16:12:34 | { id: req.body.id } |
804+
| mysql.js:6:9:6:31 | temp | mysql.js:15:62:15:65 | temp |
805+
| mysql.js:6:9:6:31 | temp | mysql.js:19:70:19:73 | temp |
806+
| mysql.js:6:16:6:31 | req.params.value | mysql.js:6:9:6:31 | temp |
807+
| mysql.js:6:16:6:31 | req.params.value | mysql.js:6:9:6:31 | temp |
808+
| mysql.js:15:62:15:65 | temp | mysql.js:15:18:15:65 | 'SELECT ... + temp |
809+
| mysql.js:15:62:15:65 | temp | mysql.js:15:18:15:65 | 'SELECT ... + temp |
810+
| mysql.js:19:70:19:73 | temp | mysql.js:19:26:19:73 | 'SELECT ... + temp |
811+
| mysql.js:19:70:19:73 | temp | mysql.js:19:26:19:73 | 'SELECT ... + temp |
795812
| pg-promise-types.ts:7:9:7:28 | taint | pg-promise-types.ts:8:17:8:21 | taint |
796813
| pg-promise-types.ts:7:9:7:28 | taint | pg-promise-types.ts:8:17:8:21 | taint |
797814
| pg-promise-types.ts:7:17:7:28 | req.params.x | pg-promise-types.ts:7:9:7:28 | taint |
@@ -978,6 +995,8 @@ edges
978995
| mongooseJsonParse.js:23:19:23:23 | query | mongooseJsonParse.js:20:30:20:43 | req.query.data | mongooseJsonParse.js:23:19:23:23 | query | This query depends on $@. | mongooseJsonParse.js:20:30:20:43 | req.query.data | a user-provided value |
979996
| mongooseModelClient.js:11:16:11:24 | { id: v } | mongooseModelClient.js:10:22:10:29 | req.body | mongooseModelClient.js:11:16:11:24 | { id: v } | This query depends on $@. | mongooseModelClient.js:10:22:10:29 | req.body | a user-provided value |
980997
| mongooseModelClient.js:12:16:12:34 | { id: req.body.id } | mongooseModelClient.js:12:22:12:29 | req.body | mongooseModelClient.js:12:16:12:34 | { id: req.body.id } | This query depends on $@. | mongooseModelClient.js:12:22:12:29 | req.body | a user-provided value |
998+
| mysql.js:15:18:15:65 | 'SELECT ... + temp | mysql.js:6:16:6:31 | req.params.value | mysql.js:15:18:15:65 | 'SELECT ... + temp | This query depends on $@. | mysql.js:6:16:6:31 | req.params.value | a user-provided value |
999+
| mysql.js:19:26:19:73 | 'SELECT ... + temp | mysql.js:6:16:6:31 | req.params.value | mysql.js:19:26:19:73 | 'SELECT ... + temp | This query depends on $@. | mysql.js:6:16:6:31 | req.params.value | a user-provided value |
9811000
| pg-promise-types.ts:8:17:8:21 | taint | pg-promise-types.ts:7:17:7:28 | req.params.x | pg-promise-types.ts:8:17:8:21 | taint | This query depends on $@. | pg-promise-types.ts:7:17:7:28 | req.params.x | a user-provided value |
9821001
| pg-promise.js:9:10:9:14 | query | pg-promise.js:7:16:7:34 | req.params.category | pg-promise.js:9:10:9:14 | query | This query depends on $@. | pg-promise.js:7:16:7:34 | req.params.category | a user-provided value |
9831002
| pg-promise.js:10:11:10:15 | query | pg-promise.js:7:16:7:34 | req.params.category | pg-promise.js:10:11:10:15 | query | This query depends on $@. | pg-promise.js:7:16:7:34 | req.params.category | a user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-089/untyped/mysql.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
const app = require("express")();
2+
const mysql = require('mysql');
3+
const pool = mysql.createPool(getConfig());
4+
5+
app.get("search", function handler(req, res) {
6+
let temp = req.params.value;
7+
pool.getConnection(function(err, connection) {
8+
connection.query({
9+
sql: 'SELECT * FROM `books` WHERE `author` = ?', // OK
10+
values: [temp]
11+
}, function(error, results, fields) {});
12+
});
13+
pool.getConnection(function(err, connection) {
14+
connection.query({
15+
sql: 'SELECT * FROM `books` WHERE `author` = ' + temp, // NOT OK
16+
}, function(error, results, fields) {});
17+
});
18+
pool.getConnection(function(err, connection) {
19+
connection.query('SELECT * FROM `books` WHERE `author` = ' + temp, // NOT OK
20+
function(error, results, fields) {});
21+
});
22+
});

0 commit comments

Comments
 (0)