Skip to content

Commit 9229962

Browse files
NapalysNapalys
committed
Add sink model for SQL injection detection in exec clients.
1 parent 032cfc1 commit 9229962

File tree

3 files changed

+42
-6
lines changed

3 files changed

+42
-6
lines changed

‎javascript/ql/lib/ext/hana-db-client.model.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/javascript-all
4+
extensible: sinkModel
5+
data:
6+
- ["@sap/hana-client", "Member[createConnection].ReturnValue.Member[exec].Argument[0]", "sql-injection"]
7+
8+
- ["hdb", "Member[createClient].ReturnValue.Member[exec].Argument[0]", "sql-injection"]

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
| graphql.js:74:46:74:64 | "{ foo" + id + " }" | graphql.js:73:14:73:25 | req.query.id | graphql.js:74:46:74:64 | "{ foo" + id + " }" | This query string depends on a $@. | graphql.js:73:14:73:25 | req.query.id | user-provided value |
1111
| graphql.js:82:14:88:8 | `{\\n ... }` | graphql.js:73:14:73:25 | req.query.id | graphql.js:82:14:88:8 | `{\\n ... }` | This query string depends on a $@. | graphql.js:73:14:73:25 | req.query.id | user-provided value |
1212
| graphql.js:118:38:118:48 | `foo ${id}` | graphql.js:117:16:117:28 | req.params.id | graphql.js:118:38:118:48 | `foo ${id}` | This query string depends on a $@. | graphql.js:117:16:117:28 | req.params.id | user-provided value |
13+
| hana.js:11:19:11:23 | query | hana.js:9:30:9:37 | req.body | hana.js:11:19:11:23 | query | This query string depends on a $@. | hana.js:9:30:9:37 | req.body | user-provided value |
14+
| hana.js:71:44:71:99 | "INSERT ... usInput | hana.js:68:24:68:31 | req.body | hana.js:71:44:71:99 | "INSERT ... usInput | This query string depends on a $@. | hana.js:68:24:68:31 | req.body | user-provided value |
15+
| hana.js:73:17:73:54 | 'select ... usInput | hana.js:68:24:68:31 | req.body | hana.js:73:17:73:54 | 'select ... usInput | This query string depends on a $@. | hana.js:68:24:68:31 | req.body | user-provided value |
16+
| hana.js:74:17:74:54 | 'select ... usInput | hana.js:68:24:68:31 | req.body | hana.js:74:17:74:54 | 'select ... usInput | This query string depends on a $@. | hana.js:68:24:68:31 | req.body | user-provided value |
1317
| html-sanitizer.js:16:9:16:59 | `SELECT ... param1 | html-sanitizer.js:13:39:13:44 | param1 | html-sanitizer.js:16:9:16:59 | `SELECT ... param1 | This query string depends on a $@. | html-sanitizer.js:13:39:13:44 | param1 | user-provided value |
1418
| json-schema-validator.js:33:22:33:26 | query | json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:33:22:33:26 | query | This query object depends on a $@. | json-schema-validator.js:25:34:25:47 | req.query.data | user-provided value |
1519
| json-schema-validator.js:35:18:35:22 | query | json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:35:18:35:22 | query | This query object depends on a $@. | json-schema-validator.js:25:34:25:47 | req.query.data | user-provided value |
@@ -152,6 +156,17 @@ edges
152156
| graphql.js:117:11:117:28 | id | graphql.js:118:45:118:46 | id | provenance | |
153157
| graphql.js:117:16:117:28 | req.params.id | graphql.js:117:11:117:28 | id | provenance | |
154158
| graphql.js:118:45:118:46 | id | graphql.js:118:38:118:48 | `foo ${id}` | provenance | |
159+
| hana.js:9:13:9:42 | maliciousInput | hana.js:10:64:10:77 | maliciousInput | provenance | |
160+
| hana.js:9:30:9:37 | req.body | hana.js:9:13:9:42 | maliciousInput | provenance | |
161+
| hana.js:10:15:10:80 | query | hana.js:11:19:11:23 | query | provenance | |
162+
| hana.js:10:64:10:77 | maliciousInput | hana.js:10:15:10:80 | query | provenance | |
163+
| hana.js:68:7:68:36 | maliciousInput | hana.js:71:86:71:99 | maliciousInput | provenance | |
164+
| hana.js:68:7:68:36 | maliciousInput | hana.js:73:41:73:54 | maliciousInput | provenance | |
165+
| hana.js:68:7:68:36 | maliciousInput | hana.js:74:41:74:54 | maliciousInput | provenance | |
166+
| hana.js:68:24:68:31 | req.body | hana.js:68:7:68:36 | maliciousInput | provenance | |
167+
| hana.js:71:86:71:99 | maliciousInput | hana.js:71:44:71:99 | "INSERT ... usInput | provenance | |
168+
| hana.js:73:41:73:54 | maliciousInput | hana.js:73:17:73:54 | 'select ... usInput | provenance | |
169+
| hana.js:74:41:74:54 | maliciousInput | hana.js:74:17:74:54 | 'select ... usInput | provenance | |
155170
| html-sanitizer.js:13:39:13:44 | param1 | html-sanitizer.js:14:18:14:23 | param1 | provenance | |
156171
| html-sanitizer.js:14:5:14:24 | param1 | html-sanitizer.js:16:54:16:59 | param1 | provenance | |
157172
| html-sanitizer.js:14:14:14:24 | xss(param1) | html-sanitizer.js:14:5:14:24 | param1 | provenance | |
@@ -504,6 +519,19 @@ nodes
504519
| graphql.js:117:16:117:28 | req.params.id | semmle.label | req.params.id |
505520
| graphql.js:118:38:118:48 | `foo ${id}` | semmle.label | `foo ${id}` |
506521
| graphql.js:118:45:118:46 | id | semmle.label | id |
522+
| hana.js:9:13:9:42 | maliciousInput | semmle.label | maliciousInput |
523+
| hana.js:9:30:9:37 | req.body | semmle.label | req.body |
524+
| hana.js:10:15:10:80 | query | semmle.label | query |
525+
| hana.js:10:64:10:77 | maliciousInput | semmle.label | maliciousInput |
526+
| hana.js:11:19:11:23 | query | semmle.label | query |
527+
| hana.js:68:7:68:36 | maliciousInput | semmle.label | maliciousInput |
528+
| hana.js:68:24:68:31 | req.body | semmle.label | req.body |
529+
| hana.js:71:44:71:99 | "INSERT ... usInput | semmle.label | "INSERT ... usInput |
530+
| hana.js:71:86:71:99 | maliciousInput | semmle.label | maliciousInput |
531+
| hana.js:73:17:73:54 | 'select ... usInput | semmle.label | 'select ... usInput |
532+
| hana.js:73:41:73:54 | maliciousInput | semmle.label | maliciousInput |
533+
| hana.js:74:17:74:54 | 'select ... usInput | semmle.label | 'select ... usInput |
534+
| hana.js:74:41:74:54 | maliciousInput | semmle.label | maliciousInput |
507535
| html-sanitizer.js:13:39:13:44 | param1 | semmle.label | param1 |
508536
| html-sanitizer.js:14:5:14:24 | param1 | semmle.label | param1 |
509537
| html-sanitizer.js:14:14:14:24 | xss(param1) | semmle.label | xss(param1) |

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ const connectionParams = {};
66
app.post('/documents/find', (req, res) => {
77
const conn = hana.createConnection();
88
conn.connect(connectionParams, (err) => {
9-
let maliciousInput = req.body.data; // $ MISSING: Source
9+
let maliciousInput = req.body.data; // $ Source
1010
const query = `SELECT * FROM Users WHERE username = '${maliciousInput}'`;
11-
conn.exec(query, (err, rows) => {}); // $ MISSING: Alert
11+
conn.exec(query, (err, rows) => {}); // $ Alert
1212
conn.disconnect();
1313
});
1414

@@ -65,13 +65,13 @@ const app2 = express();
6565

6666
app2.post('/documents/find', (req, res) => {
6767
var client = hdb.createClient(options);
68-
let maliciousInput = req.body.data; // $ MISSING: Source
68+
let maliciousInput = req.body.data; // $ Source
6969

7070
client.connect(function onconnect(err) {
71-
async.series([client.exec.bind(client, "INSERT INTO NUMBERS VALUES (1, 'one')" + maliciousInput)], function (err) {}); // $ MISSING: Alert
71+
async.series([client.exec.bind(client, "INSERT INTO NUMBERS VALUES (1, 'one')" + maliciousInput)], function (err) {}); // $ Alert
7272

73-
client.exec('select * from DUMMY' + maliciousInput, function (err, rows) {}); // $ MISSING: Alert
74-
client.exec('select * from DUMMY' + maliciousInput, options, function(err, rows) {}); // $ MISSING: Alert
73+
client.exec('select * from DUMMY' + maliciousInput, function (err, rows) {}); // $ Alert
74+
client.exec('select * from DUMMY' + maliciousInput, options, function(err, rows) {}); // $ Alert
7575

7676
client.prepare('select * from DUMMY where DUMMY = ?' + maliciousInput, function (err, statement){ // $ MISSING: Alert
7777
statement.exec([maliciousInput], function (err, rows) {}); // maliciousInput is treated as a parameter

0 commit comments

Comments
 (0)