Skip to content

Commit 342c7ab

Browse files
codeql-cicodeql-ci
authored
Merge pull request github#5301 from asgerf/js/ajv-model
Approved by erik-krogh
2 parents f9973d1 + 919ee38 commit 342c7ab

32 files changed

+657
-28
lines changed

‎javascript/change-notes/2021-03-01-ajv.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
lgtm,codescanning
2+
* The security queries now recognize the effect of JSON schema validation, and highlights
3+
cases where this validation is susceptible to denial-of-service attacks.
4+
Affects the package [ajv](https://npmjs.com/package/ajv).
5+
* A new query, `js/resource-exhaustion-from-deep-object-traversal`, has been added to the query suite,
6+
highlighting denial-of-service attacks exploiting operations that traverse deeply user-controlled objects.
7+
* The `js/xss-through-exception` query now recognizes JSON schema validation errors as a source, as they
8+
may contain part of the input data.

‎javascript/ql/src/Security/CWE-079/ExceptionXss.qhelp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
<overview>
77
<p>
8-
Directly writing exceptions to a webpage without sanitization allows for a cross-site scripting
9-
vulnerability if the value of the exception can be influenced by a user.
8+
Directly writing error messages to a webpage without sanitization allows for a cross-site scripting
9+
vulnerability if parts of the error message can be influenced by a user.
1010
</p>
1111
</overview>
1212

@@ -27,6 +27,19 @@ leaving the website vulnerable to cross-site scripting.
2727
<sample src="examples/ExceptionXss.js" />
2828
</example>
2929

30+
<example>
31+
<p>
32+
This second example shows an input being validated using the JSON schema validator <code>ajv</code>,
33+
and in case of an error, the error message is sent directly back in the response.
34+
</p>
35+
<sample src="examples/ExceptionXssAjv.js" />
36+
<p>
37+
This is unsafe, because the error message can contain parts of the input.
38+
For example, the input <code>{'&lt;img src=x onerror=alert(1)&gt;': 'foo'}</code> will generate the error
39+
<code>data/&lt;img src=x onerror=alert(1)&gt; should be number</code>, causing reflected XSS.
40+
</p>
41+
</example>
42+
3043
<references>
3144
<li>
3245
OWASP:

‎javascript/ql/src/Security/CWE-079/ExceptionXss.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,4 +19,4 @@ from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
1919
where cfg.hasFlowPath(source, sink)
2020
select sink.getNode(), source, sink,
2121
"$@ is reinterpreted as HTML without escaping meta-characters.", source.getNode(),
22-
"Exception text"
22+
source.getNode().(Source).getDescription()

‎javascript/ql/src/Security/CWE-079/examples/ExceptionXssAjv.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import express from 'express';
2+
import Ajv from 'ajv';
3+
4+
let app = express();
5+
let ajv = new Ajv();
6+
7+
ajv.addSchema({type: 'object', additionalProperties: {type: 'number'}}, 'pollData');
8+
9+
app.post('/polldata', (req, res) => {
10+
if (!ajv.validate('pollData', req.body)) {
11+
res.send(ajv.errorsText());
12+
}
13+
});

‎javascript/ql/src/Security/CWE-400/DeepObjectResourceExhaustion.qhelp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Processing user-controlled data with a method that allocates excessive amounts
9+
of memory can lead to denial of service.
10+
</p>
11+
12+
<p>
13+
If the JSON schema validation library <code>ajv</code> is configured with
14+
<code>allErrors: true</code> there is no limit to how many error objects
15+
will be allocated. An attacker can exploit this by sending an object that
16+
deliberately contains a huge number of errors, and in some cases, with
17+
longer and longer error messages. This can cause the service to become
18+
unresponsive due to the slow error-checking process.
19+
</p>
20+
</overview>
21+
22+
<recommendation>
23+
<p>
24+
Do not use <code>allErrors: true</code> in production.
25+
</p>
26+
</recommendation>
27+
28+
<example>
29+
<p>
30+
In the example below, the user-submitted object <code>req.body</code> is
31+
validated using <code>ajv</code> and <code>allErrors: true</code>:
32+
</p>
33+
34+
<sample src="examples/DeepObjectResourceExhaustion.js"/>
35+
36+
<p>
37+
Although this ensures that <code>req.body</code> conforms to the schema,
38+
the validation itself could be vulnerable to a denial-of-service attack.
39+
An attacker could send an object containing so many errors that the server
40+
runs out of memory.
41+
</p>
42+
43+
<p>
44+
A solution is to not pass in <code>allErrors: true</code>, which means
45+
<code>ajv</code> will only report the first error, not all of them:
46+
</p>
47+
48+
<sample src="examples/DeepObjectResourceExhaustion_fixed.js"/>
49+
</example>
50+
51+
<references>
52+
<li>Ajv documentation: <a href="https://github.com/ajv-validator/ajv/blob/master/docs/security.md#untrusted-schemas">security considerations</a>
53+
</li>
54+
</references>
55+
</qhelp>

‎javascript/ql/src/Security/CWE-400/DeepObjectResourceExhaustion.ql

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @name Resources exhaustion from deep object traversal
3+
* @description Processing user-controlled object hierarchies inefficiently can lead to denial of service.
4+
* @kind path-problem
5+
* @problem.severity warning
6+
* @precision high
7+
* @id js/resource-exhaustion-from-deep-object-traversal
8+
* @tags security
9+
* external/cwe/cwe-400
10+
*/
11+
12+
import javascript
13+
import DataFlow::PathGraph
14+
import semmle.javascript.security.dataflow.DeepObjectResourceExhaustion::DeepObjectResourceExhaustion
15+
16+
from
17+
Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node link,
18+
string reason
19+
where
20+
cfg.hasFlowPath(source, sink) and
21+
sink.getNode().(Sink).hasReason(link, reason)
22+
select sink, source, sink, "Denial of service caused by processing user input from $@ with $@.",
23+
source.getNode(), "here", link, reason

‎javascript/ql/src/Security/CWE-400/examples/DeepObjectResourceExhaustion.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import express from 'express';
2+
import Ajv from 'ajv';
3+
4+
let ajv = new Ajv({ allErrors: true });
5+
ajv.addSchema(require('./input-schema'), 'input');
6+
7+
var app = express();
8+
app.get('/user/:id', function(req, res) {
9+
if (!ajv.validate('input', req.body)) {
10+
res.end(ajv.errorsText());
11+
return;
12+
}
13+
// ...
14+
});

‎javascript/ql/src/Security/CWE-400/examples/DeepObjectResourceExhaustion_fixed.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import express from 'express';
2+
import Ajv from 'ajv';
3+
4+
let ajv = new Ajv({ allErrors: process.env['REST_DEBUG'] });
5+
ajv.addSchema(require('./input-schema'), 'input');
6+
7+
var app = express();
8+
app.get('/user/:id', function(req, res) {
9+
if (!ajv.validate('input', req.body)) {
10+
res.end(ajv.errorsText());
11+
return;
12+
}
13+
// ...
14+
});

‎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: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
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+
/** A data flow node that is used a JSON schema. */
21+
abstract class SchemaRoot extends DataFlow::Node { }
22+
23+
/** An object literal with a `$schema` property indicating it is the root of a JSON schema. */
24+
private class SchemaNodeByTag extends SchemaRoot, DataFlow::ObjectLiteralNode {
25+
SchemaNodeByTag() {
26+
getAPropertyWrite("$schema").getRhs().getStringValue().matches("%//json-schema.org%")
27+
}
28+
}
29+
30+
/** Gets a data flow node that is part of a JSON schema. */
31+
private DataFlow::SourceNode getAPartOfJsonSchema(DataFlow::TypeBackTracker t) {
32+
t.start() and
33+
result = any(SchemaRoot n).getALocalSource()
34+
or
35+
result = getAPartOfJsonSchema(t.continue()).getAPropertySource()
36+
or
37+
exists(DataFlow::TypeBackTracker t2 | result = getAPartOfJsonSchema(t2).backtrack(t2, t))
38+
}
39+
40+
/** Gets a data flow node that is part of a JSON schema. */
41+
DataFlow::SourceNode getAPartOfJsonSchema() {
42+
result = getAPartOfJsonSchema(DataFlow::TypeBackTracker::end())
43+
}
44+
45+
/** Provides a model of the `ajv` library. */
46+
module Ajv {
47+
/** A method on `Ajv` that returns `this`. */
48+
private string chainedMethod() {
49+
result =
50+
["addSchema", "addMetaSchema", "removeSchema", "addFormat", "addKeyword", "removeKeyword"]
51+
}
52+
53+
/** An instance of `ajv`. */
54+
class Instance extends API::InvokeNode {
55+
Instance() { this = API::moduleImport("ajv").getAnInstantiation() }
56+
57+
/** Gets the data flow node holding the options passed to this `Ajv` instance. */
58+
DataFlow::Node getOptionsArg() { result = getArgument(0) }
59+
60+
/** Gets an API node that refers to this object. */
61+
API::Node ref() {
62+
result = getReturn()
63+
or
64+
result = ref().getMember(chainedMethod()).getReturn()
65+
}
66+
67+
/**
68+
* Gets an API node for a function produced by `new Ajv().compile()` or similar.
69+
*
70+
* Note that this does not include the instance method `new Ajv().validate` as its
71+
* signature is different.
72+
*/
73+
API::Node getAValidationFunction() {
74+
result = ref().getMember(["compile", "getSchema"]).getReturn()
75+
or
76+
result = ref().getMember("compileAsync").getPromised()
77+
}
78+
79+
/**
80+
* Gets an API node that refers to an error produced by this Ajv instance.
81+
*/
82+
API::Node getAValidationError() {
83+
exists(API::Node base | base = [ref(), getAValidationFunction()] |
84+
result = base.getMember("errors")
85+
or
86+
result = base.getMember("errorsText").getReturn()
87+
)
88+
}
89+
}
90+
91+
/** A call to the `validate` method of `ajv`. */
92+
class AjvValidationCall extends ValidationCall {
93+
Instance instance;
94+
int argIndex;
95+
96+
AjvValidationCall() {
97+
this = instance.ref().getMember("validate").getACall() and argIndex = 1
98+
or
99+
this = instance.getAValidationFunction().getACall() and argIndex = 0
100+
}
101+
102+
override DataFlow::Node getInput() { result = getArgument(argIndex) }
103+
104+
/** Gets the argument holding additional options to the call. */
105+
DataFlow::Node getOwnOptionsArg() { result = getArgument(argIndex + 1) }
106+
107+
/** Gets a data flow passed as the extra options to this validation call or to the underlying `Ajv` instance. */
108+
DataFlow::Node getAnOptionsArg() {
109+
result = getOwnOptionsArg()
110+
or
111+
result = instance.getOptionsArg()
112+
}
113+
114+
/** Gets the ajv instance doing the validation. */
115+
Instance getAjvInstance() { result = instance }
116+
}
117+
118+
private class AjvSchemaNode extends SchemaRoot {
119+
AjvSchemaNode() {
120+
this =
121+
any(Instance i)
122+
.ref()
123+
.getMember(["addSchema", "validate", "compile", "compileAsync"])
124+
.getParameter(0)
125+
.getARhs()
126+
}
127+
}
128+
}
129+
}

0 commit comments

Comments
 (0)