Skip to content

Commit be7abed

Browse files
erik-krogherik-krogh
committed
add model for the joi library
1 parent d254524 commit be7abed

File tree

8 files changed

+131
-3
lines changed

8 files changed

+131
-3
lines changed

‎javascript/change-notes/2021-06-07-joi.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
lgtm,codescanning
2+
* The security queries now recognize the JSON schema validation from the [joi](https://npmjs.org/package/joi) library.
3+
Affected packages are
4+
[joi](https://npmjs.org/package/joi)

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

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ import javascript
88
* Provides classes and predicates for working with JSON schema libraries.
99
*/
1010
module JsonSchema {
11-
/** A call that validates an input against a JSON schema. */
12-
abstract class ValidationCall extends DataFlow::CallNode {
11+
/** A node that validates an input against a JSON schema. */
12+
abstract class ValidationCall extends DataFlow::Node {
1313
/** Gets the data flow node whose value is being validated. */
1414
abstract DataFlow::Node getInput();
1515

@@ -89,7 +89,7 @@ module JsonSchema {
8989
}
9090

9191
/** A call to the `validate` method of `ajv`. */
92-
class AjvValidationCall extends ValidationCall {
92+
class AjvValidationCall extends ValidationCall, DataFlow::CallNode {
9393
Instance instance;
9494
int argIndex;
9595

@@ -126,4 +126,55 @@ module JsonSchema {
126126
}
127127
}
128128
}
129+
130+
/** Provides a model for working with the [`joi`](https://npmjs.org/package/joi) library. */
131+
module Joi {
132+
/** A schema created using `joi.object()` or other schemas that might refer an object schema. */
133+
private API::Node objectSchema() {
134+
// A call that creates a schema that might be an object schema.
135+
result =
136+
API::moduleImport("joi")
137+
.getMember([
138+
"object", "alternatives", "all", "link", "compile", "allow", "valid", "when",
139+
"build", "options"
140+
])
141+
.getReturn()
142+
or
143+
// A call to a schema that returns another schema.
144+
// Read from the [index.d.ts](https://github.com/sideway/joi/blob/master/lib/index.d.ts) file.
145+
result =
146+
objectSchema()
147+
.getMember([
148+
// AnySchema
149+
"allow", "alter", "bind", "cache", "cast", "concat", "default", "description",
150+
"disallow", "empty", "equal", "error", "example", "exist", "external", "failover",
151+
"forbidden", "fork", "id", "invalid", "keep", "label", "message", "messages",
152+
"meta", "not", "note", "only", "optional", "options", "prefs", "preferences",
153+
"presence", "raw", "required", "rule", "shared", "strict", "strip", "tag", "tailor",
154+
"unit", "valid", "warn", "warning", "when",
155+
// ObjectSchema
156+
"and", "append", "assert", "instance", "keys", "length", "max", "min", "nand", "or",
157+
"oxor", "pattern", "ref", "regex", "rename", "schema", "unknown", "with", "without",
158+
"xor"
159+
])
160+
.getReturn()
161+
}
162+
163+
/**
164+
* A read of the `error` property from a validation result, seen as a `ValidationCall`.
165+
* If `error` exists, then the validation failed.
166+
*/
167+
class JoiValidationErrorRead extends ValidationCall {
168+
API::CallNode validateCall;
169+
170+
JoiValidationErrorRead() {
171+
validateCall = objectSchema().getMember("validate").getACall() and
172+
this = validateCall.getReturn().getMember("error").getAnImmediateUse()
173+
}
174+
175+
override DataFlow::Node getInput() { result = validateCall.getArgument(0) }
176+
177+
override boolean getPolarity() { result = false }
178+
}
179+
}
129180
}

‎javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,8 @@ module ExceptionXss {
617617
private class JsonSchemaValidationError extends Source {
618618
JsonSchemaValidationError() {
619619
this = any(JsonSchema::Ajv::Instance i).getAValidationError().getAnImmediateUse()
620+
or
621+
this = any(JsonSchema::Joi::JoiValidationErrorRead r)
620622
}
621623

622624
override string getDescription() { result = "JSON schema validation error" }

‎javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss/ExceptionXss.expected

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,9 @@ nodes
22
| ajv.js:11:18:11:33 | ajv.errorsText() |
33
| ajv.js:11:18:11:33 | ajv.errorsText() |
44
| ajv.js:11:18:11:33 | ajv.errorsText() |
5+
| ajv.js:24:18:24:26 | val.error |
6+
| ajv.js:24:18:24:26 | val.error |
7+
| ajv.js:24:18:24:26 | val.error |
58
| exception-xss.js:2:6:2:28 | foo |
69
| exception-xss.js:2:12:2:28 | document.location |
710
| exception-xss.js:2:12:2:28 | document.location |
@@ -89,6 +92,7 @@ nodes
8992
| exception-xss.js:182:19:182:23 | error |
9093
edges
9194
| ajv.js:11:18:11:33 | ajv.errorsText() | ajv.js:11:18:11:33 | ajv.errorsText() |
95+
| ajv.js:24:18:24:26 | val.error | ajv.js:24:18:24:26 | val.error |
9296
| exception-xss.js:2:6:2:28 | foo | exception-xss.js:9:11:9:13 | foo |
9397
| exception-xss.js:2:6:2:28 | foo | exception-xss.js:15:9:15:11 | foo |
9498
| exception-xss.js:2:6:2:28 | foo | exception-xss.js:21:11:21:13 | foo |
@@ -170,6 +174,7 @@ edges
170174
| exception-xss.js:180:26:180:30 | error | exception-xss.js:182:19:182:23 | error |
171175
#select
172176
| ajv.js:11:18:11:33 | ajv.errorsText() | ajv.js:11:18:11:33 | ajv.errorsText() | ajv.js:11:18:11:33 | ajv.errorsText() | $@ is reinterpreted as HTML without escaping meta-characters. | ajv.js:11:18:11:33 | ajv.errorsText() | JSON schema validation error |
177+
| ajv.js:24:18:24:26 | val.error | ajv.js:24:18:24:26 | val.error | ajv.js:24:18:24:26 | val.error | $@ is reinterpreted as HTML without escaping meta-characters. | ajv.js:24:18:24:26 | val.error | JSON schema validation error |
173178
| exception-xss.js:11:18:11:18 | e | exception-xss.js:2:12:2:28 | document.location | exception-xss.js:11:18:11:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:2:12:2:28 | document.location | Exception text |
174179
| exception-xss.js:17:18:17:18 | e | exception-xss.js:2:12:2:28 | document.location | exception-xss.js:17:18:17:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:2:12:2:28 | document.location | Exception text |
175180
| exception-xss.js:23:18:23:18 | e | exception-xss.js:2:12:2:28 | document.location | exception-xss.js:23:18:23:18 | e | $@ is reinterpreted as HTML without escaping meta-characters. | exception-xss.js:2:12:2:28 | document.location | Exception text |

‎javascript/ql/test/query-tests/Security/CWE-079/ExceptionXss/ajv.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,16 @@ app.post('/polldata', (req, res) => {
1111
res.send(ajv.errorsText()); // NOT OK
1212
}
1313
});
14+
15+
const joi = require("joi");
16+
const joiSchema = joi.object().keys({
17+
name: joi.string().required(),
18+
age: joi.number().required()
19+
}).with('name', 'age');
20+
21+
app.post('/votedata', (req, res) => {
22+
const val = joiSchema.validate(req.body);
23+
if (val.error) {
24+
res.send(val.error); // NOT OK
25+
}
26+
});

‎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
@@ -2,6 +2,10 @@
22
| json-schema-validator.js:30:13:30:27 | doc.find(query) |
33
| json-schema-validator.js:33:13:33:27 | doc.find(query) |
44
| json-schema-validator.js:35:9:35:23 | doc.find(query) |
5+
| json-schema-validator.js:53:13:53:27 | doc.find(query) |
6+
| json-schema-validator.js:55:13:55:27 | doc.find(query) |
7+
| json-schema-validator.js:59:13:59:27 | doc.find(query) |
8+
| json-schema-validator.js:61:13:61:27 | doc.find(query) |
59
| marsdb-flow-to.js:14:3:14:22 | db.myDoc.find(query) |
610
| marsdb.js:16:3:16:17 | doc.find(query) |
711
| minimongo.js:18:3:18:17 | doc.find(query) |

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,16 @@ nodes
77
| json-schema-validator.js:33:22:33:26 | query |
88
| json-schema-validator.js:35:18:35:22 | query |
99
| json-schema-validator.js:35:18:35:22 | query |
10+
| json-schema-validator.js:50:15:50:48 | query |
11+
| json-schema-validator.js:50:23:50:48 | JSON.pa ... y.data) |
12+
| json-schema-validator.js:50:34:50:47 | req.query.data |
13+
| json-schema-validator.js:50:34:50:47 | req.query.data |
14+
| json-schema-validator.js:55:22:55:26 | query |
15+
| json-schema-validator.js:55:22:55:26 | query |
16+
| json-schema-validator.js:59:22:59:26 | query |
17+
| json-schema-validator.js:59:22:59:26 | query |
18+
| json-schema-validator.js:61:22:61:26 | query |
19+
| json-schema-validator.js:61:22:61:26 | query |
1020
| marsdb-flow-to.js:10:9:10:18 | query |
1121
| marsdb-flow-to.js:10:17:10:18 | {} |
1222
| marsdb-flow-to.js:11:17:11:24 | req.body |
@@ -329,6 +339,15 @@ edges
329339
| json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) | json-schema-validator.js:25:15:25:48 | query |
330340
| json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
331341
| json-schema-validator.js:25:34:25:47 | req.query.data | json-schema-validator.js:25:23:25:48 | JSON.pa ... y.data) |
342+
| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:55:22:55:26 | query |
343+
| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:55:22:55:26 | query |
344+
| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:59:22:59:26 | query |
345+
| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:59:22:59:26 | query |
346+
| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:61:22:61:26 | query |
347+
| json-schema-validator.js:50:15:50:48 | query | json-schema-validator.js:61:22:61:26 | query |
348+
| json-schema-validator.js:50:23:50:48 | JSON.pa ... y.data) | json-schema-validator.js:50:15:50:48 | query |
349+
| json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:50:23:50:48 | JSON.pa ... y.data) |
350+
| json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:50:23:50:48 | JSON.pa ... y.data) |
332351
| marsdb-flow-to.js:10:9:10:18 | query | marsdb-flow-to.js:14:17:14:21 | query |
333352
| marsdb-flow-to.js:10:9:10:18 | query | marsdb-flow-to.js:14:17:14:21 | query |
334353
| marsdb-flow-to.js:10:17:10:18 | {} | marsdb-flow-to.js:10:9:10:18 | query |
@@ -723,6 +742,9 @@ edges
723742
#select
724743
| 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 |
725744
| 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 |
745+
| json-schema-validator.js:55:22:55:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:55:22:55:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |
746+
| json-schema-validator.js:59:22:59:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:59:22:59:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |
747+
| json-schema-validator.js:61:22:61:26 | query | json-schema-validator.js:50:34:50:47 | req.query.data | json-schema-validator.js:61:22:61:26 | query | This query depends on $@. | json-schema-validator.js:50:34:50:47 | req.query.data | a user-provided value |
726748
| 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 |
727749
| 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 |
728750
| 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: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,30 @@ app.post('/documents/find', (req, res) => {
3535
doc.find(query); // NOT OK
3636
});
3737
});
38+
39+
import Joi from 'joi';
40+
41+
const joiSchema = Joi.object({
42+
date: Joi.string().required(),
43+
title: Joi.string().required()
44+
}).with('date', 'title');
45+
46+
app.post('/documents/insert', (req, res) => {
47+
MongoClient.connect('mongodb://localhost:27017/test', async (err, db) => {
48+
let doc = db.collection('doc');
49+
50+
const query = JSON.parse(req.query.data);
51+
const validate = joiSchema.validate(query);
52+
if (!validate.error) {
53+
doc.find(query); // OK
54+
} else {
55+
doc.find(query); // NOT OK
56+
}
57+
try {
58+
await joiSchema.validateAsync(query);
59+
doc.find(query); // OK - but still flagged [INCONSISTENCY]
60+
} catch (e) {
61+
doc.find(query); // NOT OK
62+
}
63+
});
64+
});

0 commit comments

Comments
 (0)