Skip to content

Commit b978359

Browse files
asgerfasgerf
committed
JS: Add schema validation as TaintedObject sanitizer
1 parent 79839d2 commit b978359

File tree

6 files changed

+147
-0
lines changed

6 files changed

+147
-0
lines changed

‎javascript/ql/src/javascript.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import semmle.javascript.InclusionTests
3636
import semmle.javascript.JSDoc
3737
import semmle.javascript.JSON
3838
import semmle.javascript.JsonParsers
39+
import semmle.javascript.JsonSchema
3940
import semmle.javascript.JsonStringifiers
4041
import semmle.javascript.JSX
4142
import semmle.javascript.Lines

‎javascript/ql/src/semmle/javascript/JsonSchema.qll

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/**
2+
* Provides classes and predicates for working with JSON schema libraries.
3+
*/
4+
5+
import javascript
6+
7+
/**
8+
* Provides classes and predicates for working with JSON schema libraries.
9+
*/
10+
module JsonSchema {
11+
/** A call that validates an input against a JSON schema. */
12+
abstract class ValidationCall extends DataFlow::CallNode {
13+
/** Gets the data flow node whose value is being validated. */
14+
abstract DataFlow::Node getInput();
15+
16+
/** Gets the return value that indicates successful validation. */
17+
boolean getPolarity() { result = true }
18+
}
19+
20+
/** Provides a model of the `ajv` library. */
21+
module Ajv {
22+
/** A method on `Ajv` that returns `this`. */
23+
private string chainedMethod() {
24+
result =
25+
["addSchema", "addMetaSchema", "removeSchema", "addFormat", "addKeyword", "removeKeyword"]
26+
}
27+
28+
/** An instance of `ajv`. */
29+
class Instance extends API::InvokeNode {
30+
Instance() { this = API::moduleImport("ajv").getAnInstantiation() }
31+
32+
/** Gets the data flow node holding the options passed to this `Ajv` instance. */
33+
DataFlow::Node getOptionsArg() { result = getArgument(0) }
34+
35+
/** Gets an API node that refers to this object. */
36+
API::Node ref() {
37+
result = getReturn()
38+
or
39+
result = ref().getMember(chainedMethod()).getReturn()
40+
}
41+
}
42+
43+
/** A call to the `validate` method of `ajv`. */
44+
class AjvValidationCall extends ValidationCall {
45+
Instance instance;
46+
int argIndex;
47+
48+
AjvValidationCall() {
49+
this = instance.ref().getMember("validate").getACall() and argIndex = 1
50+
or
51+
this = instance.ref().getMember(["compile", "getSchema"]).getReturn().getACall() and
52+
argIndex = 0
53+
or
54+
this = instance.ref().getMember("compileAsync").getPromised().getACall() and argIndex = 0
55+
}
56+
57+
override DataFlow::Node getInput() { result = getArgument(argIndex) }
58+
59+
/** Gets the argument holding additional options to the call. */
60+
DataFlow::Node getOwnOptionsArg() { result = getArgument(argIndex + 1) }
61+
62+
/** Gets a data flow passed as the extra options to this validation call or to the underlying `Ajv` instance. */
63+
DataFlow::Node getAnOptionsArg() {
64+
result = getOwnOptionsArg()
65+
or
66+
result = instance.getOptionsArg()
67+
}
68+
69+
/** Gets the ajv instance doing the validation. */
70+
Instance getAjvInstance() { result = instance }
71+
}
72+
}
73+
}

‎javascript/ql/src/semmle/javascript/security/TaintedObject.qll

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,4 +120,19 @@ module TaintedObject {
120120
label = label()
121121
}
122122
}
123+
124+
/**
125+
* A sanitizer guard that validates an input against a JSON schema.
126+
*/
127+
private class JsonSchemaValidationGuard extends SanitizerGuard {
128+
JsonSchema::ValidationCall call;
129+
130+
JsonSchemaValidationGuard() { this = call }
131+
132+
override predicate sanitizes(boolean outcome, Expr e, FlowLabel label) {
133+
outcome = call.getPolarity() and
134+
e = call.getInput().asExpr() and
135+
label = label()
136+
}
137+
}
123138
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
| json-schema-validator.js:27:13:27:27 | doc.find(query) |
2+
| json-schema-validator.js:30:13:30:27 | doc.find(query) |
3+
| json-schema-validator.js:33:13:33:27 | doc.find(query) |
4+
| json-schema-validator.js:35:9:35:23 | doc.find(query) |
15
| marsdb-flow-to.js:14:3:14:22 | db.myDoc.find(query) |
26
| marsdb.js:16:3:16:17 | doc.find(query) |
37
| minimongo.js:18:3:18:17 | doc.find(query) |

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,12 @@
11
nodes
2+
| json-schema-validator.js:25:15:25:48 | query |
3+
| json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
4+
| json-schema-validator.js:25:34:25:47 | req.query.data |
5+
| json-schema-validator.js:25:34:25:47 | req.query.data |
6+
| json-schema-validator.js:33:22:33:26 | query |
7+
| json-schema-validator.js:33:22:33:26 | query |
8+
| json-schema-validator.js:35:18:35:22 | query |
9+
| json-schema-validator.js:35:18:35:22 | query |
210
| marsdb-flow-to.js:10:9:10:18 | query |
311
| marsdb-flow-to.js:10:17:10:18 | {} |
412
| marsdb-flow-to.js:11:17:11:24 | req.body |
@@ -250,6 +258,13 @@ nodes
250258
| tst.js:10:46:10:58 | req.params.id |
251259
| tst.js:10:46:10:58 | req.params.id |
252260
edges
261+
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:33:22:33:26 | query |
262+
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:33:22:33:26 | query |
263+
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:35:18:35:22 | query |
264+
| json-schema-validator.js:25:15:25:48 | query | json-schema-validator.js:35:18:35:22 | query |
265+
| json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) | json-schema-validator.js:25:15:25:48 | query |
266+
| json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
267+
| json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
253268
| marsdb-flow-to.js:10:9:10:18 | query | marsdb-flow-to.js:14:17:14:21 | query |
254269
| marsdb-flow-to.js:10:9:10:18 | query | marsdb-flow-to.js:14:17:14:21 | query |
255270
| marsdb-flow-to.js:10:17:10:18 | {} | marsdb-flow-to.js:10:9:10:18 | query |
@@ -586,6 +601,8 @@ edges
586601
| tst.js:10:46:10:58 | req.params.id | tst.js:10:10:10:64 | 'SELECT ... d + '"' |
587602
| tst.js:10:46:10:58 | req.params.id | tst.js:10:10:10:64 | 'SELECT ... d + '"' |
588603
#select
604+
| 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 depends on $@. | json-schema-validator.js:25:34:25:47 | req.query.data | a user-provided value |
605+
| 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 depends on $@. | json-schema-validator.js:25:34:25:47 | req.query.data | a user-provided value |
589606
| marsdb-flow-to.js:14:17:14:21 | query | marsdb-flow-to.js:11:17:11:24 | req.body | marsdb-flow-to.js:14:17:14:21 | query | This query depends on $@. | marsdb-flow-to.js:11:17:11:24 | req.body | a user-provided value |
590607
| marsdb.js:16:12:16:16 | query | marsdb.js:13:17:13:24 | req.body | marsdb.js:16:12:16:16 | query | This query depends on $@. | marsdb.js:13:17:13:24 | req.body | a user-provided value |
591608
| minimongo.js:18:12:18:16 | query | minimongo.js:15:17:15:24 | req.body | minimongo.js:18:12:18:16 | query | This query depends on $@. | minimongo.js:15:17:15:24 | req.body | a user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-089/untyped/json-schema-validator.js

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import Ajv from 'ajv';
2+
import express from 'express';
3+
import { MongoClient } from 'mongodb';
4+
5+
const app = express();
6+
7+
const schema = {
8+
type: 'object',
9+
properties: {
10+
date: { type: 'string' },
11+
title: { type: 'string' },
12+
},
13+
};
14+
const ajv = new Ajv();
15+
const checkSchema = ajv.compile(schema);
16+
17+
function validate(x) {
18+
return x != null;
19+
}
20+
21+
app.post('/documents/find', (req, res) => {
22+
MongoClient.connect('mongodb://localhost:27017/test', (err, db) => {
23+
let doc = db.collection('doc');
24+
25+
const query = JSON.parse(req.query.data);
26+
if (checkSchema(query)) {
27+
doc.find(query); // OK
28+
}
29+
if (ajv.validate(schema, query)) {
30+
doc.find(query); // OK
31+
}
32+
if (validate(query)) {
33+
doc.find(query); // NOT OK - validate() doesn't sanitize
34+
}
35+
doc.find(query); // NOT OK
36+
});
37+
});

0 commit comments

Comments
 (0)