Skip to content

Commit d65e6bb

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#6035 from erik-krogh/joi
Approved by asgerf
2 parents 6bdd7df + 3e171ad commit d65e6bb

File tree

9 files changed

+144
-3
lines changed

9 files changed

+144
-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: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,20 @@ module JsonSchema {
1313
/** Gets the data flow node whose value is being validated. */
1414
abstract DataFlow::Node getInput();
1515

16-
/** Gets the return value that indicates successful validation. */
16+
/**
17+
* Gets the return value that indicates successful validation, if any.
18+
*
19+
* Has no result if the return value from this call does not directly
20+
* indicate success.
21+
*/
1722
boolean getPolarity() { result = true }
23+
24+
/**
25+
* Gets a value that indicates whether the validation was successful.
26+
*/
27+
DataFlow::Node getAValidationResultAccess(boolean polarity) {
28+
result = this and polarity = getPolarity()
29+
}
1830
}
1931

2032
/** A data flow node that is used a JSON schema. */
@@ -126,4 +138,55 @@ module JsonSchema {
126138
}
127139
}
128140
}
141+
142+
/** Provides a model for working with the [`joi`](https://npmjs.org/package/joi) library. */
143+
module Joi {
144+
/** A schema created using `joi.object()` or other schemas that might refer an object schema. */
145+
private API::Node objectSchema() {
146+
// A call that creates a schema that might be an object schema.
147+
result =
148+
API::moduleImport("joi")
149+
.getMember([
150+
"object", "alternatives", "all", "link", "compile", "allow", "valid", "when",
151+
"build", "options"
152+
])
153+
.getReturn()
154+
or
155+
// A call to a schema that returns another schema.
156+
// Read from the [index.d.ts](https://github.com/sideway/joi/blob/master/lib/index.d.ts) file.
157+
result =
158+
objectSchema()
159+
.getMember([
160+
// AnySchema
161+
"allow", "alter", "bind", "cache", "cast", "concat", "default", "description",
162+
"disallow", "empty", "equal", "error", "example", "exist", "external", "failover",
163+
"forbidden", "fork", "id", "invalid", "keep", "label", "message", "messages",
164+
"meta", "not", "note", "only", "optional", "options", "prefs", "preferences",
165+
"presence", "raw", "required", "rule", "shared", "strict", "strip", "tag", "tailor",
166+
"unit", "valid", "warn", "warning", "when",
167+
// ObjectSchema
168+
"and", "append", "assert", "instance", "keys", "length", "max", "min", "nand", "or",
169+
"oxor", "pattern", "ref", "regex", "rename", "schema", "unknown", "with", "without",
170+
"xor"
171+
])
172+
.getReturn()
173+
}
174+
175+
/**
176+
* A call to the `validate` method from the [`joi`](https://npmjs.org/package/joi) library.
177+
* The `error` property in the result indicates whether the validation was successful.
178+
*/
179+
class JoiValidationErrorRead extends ValidationCall, API::CallNode {
180+
JoiValidationErrorRead() { this = objectSchema().getMember("validate").getACall() }
181+
182+
override DataFlow::Node getInput() { result = this.getArgument(0) }
183+
184+
override boolean getPolarity() { none() }
185+
186+
override DataFlow::Node getAValidationResultAccess(boolean polarity) {
187+
result = this.getReturn().getMember("error").getAnImmediateUse() and
188+
polarity = false
189+
}
190+
}
191+
}
129192
}

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,11 +115,12 @@ module TaintedObject {
115115
*/
116116
private class JsonSchemaValidationGuard extends SanitizerGuard {
117117
JsonSchema::ValidationCall call;
118+
boolean polarity;
118119

119-
JsonSchemaValidationGuard() { this = call }
120+
JsonSchemaValidationGuard() { this = call.getAValidationResultAccess(polarity) }
120121

121122
override predicate sanitizes(boolean outcome, Expr e, FlowLabel label) {
122-
outcome = call.getPolarity() and
123+
outcome = polarity and
123124
e = call.getInput().asExpr() and
124125
label = label()
125126
}

‎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).getAValidationResultAccess(_)
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)