Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions javascript/change-notes/2021-03-01-ajv.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
lgtm,codescanning
* The security queries now recognize the effect of JSON schema validation, and highlights
cases where this validation is susceptible to denial-of-service attacks.
Affects the package [ajv](https://npmjs.com/package/ajv).
* A new query, `js/resource-exhaustion-from-deep-object-traversal`, has been added to the query suite,
highlighting denial-of-service attacks exploiting operations that traverse deeply user-controlled objects.
* The `js/xss-through-exception` query now recognizes JSON schema validation errors as a source, as they
may contain part of the input data.
17 changes: 15 additions & 2 deletions javascript/ql/src/Security/CWE-079/ExceptionXss.qhelp
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@

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

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

<example>
<p>
This second example shows an input being validated using the JSON schema validator <code>ajv</code>,
and in case of an error, the error message is sent directly back in the response.
</p>
<sample src="examples/ExceptionXssAjv.js" />
<p>
This is unsafe, because the error message can contain parts of the input.
For example, the input <code>{'&lt;img src=x onerror=alert(1)&gt;': 'foo'}</code> will generate the error
<code>data/&lt;img src=x onerror=alert(1)&gt; should be number</code>, causing reflected XSS.
</p>
</example>

<references>
<li>
OWASP:
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/src/Security/CWE-079/ExceptionXss.ql
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink.getNode(), source, sink,
"$@ is reinterpreted as HTML without escaping meta-characters.", source.getNode(),
"Exception text"
source.getNode().(Source).getDescription()
13 changes: 13 additions & 0 deletions javascript/ql/src/Security/CWE-079/examples/ExceptionXssAjv.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import express from 'express';
import Ajv from 'ajv';

let app = express();
let ajv = new Ajv();

ajv.addSchema({type: 'object', additionalProperties: {type: 'number'}}, 'pollData');

app.post('/polldata', (req, res) => {
if (!ajv.validate('pollData', req.body)) {
res.send(ajv.errorsText());
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>

<overview>
<p>
Processing user-controlled data with a method that allocates excessive amounts
of memory can lead to denial of service.
</p>

<p>
If the JSON schema validation library <code>ajv</code> is configured with
<code>allErrors: true</code> there is no limit to how many error objects
will be allocated. An attacker can exploit this by sending an object that
deliberately contains a huge number of errors, and in some cases, with
longer and longer error messages. This can cause the service to become
unresponsive due to the slow error-checking process.
</p>
</overview>

<recommendation>
<p>
Do not use <code>allErrors: true</code> in production.
</p>
</recommendation>

<example>
<p>
In the example below, the user-submitted object <code>req.body</code> is
validated using <code>ajv</code> and <code>allErrors: true</code>:
</p>

<sample src="examples/DeepObjectResourceExhaustion.js"/>

<p>
Although this ensures that <code>req.body</code> conforms to the schema,
the validation itself could be vulnerable to a denial-of-service attack.
An attacker could send an object containing so many errors that the server
runs out of memory.
</p>

<p>
A solution is to not pass in <code>allErrors: true</code>, which means
<code>ajv</code> will only report the first error, not all of them:
</p>

<sample src="examples/DeepObjectResourceExhaustion_fixed.js"/>
</example>

<references>
<li>Ajv documentation: <a href="https://github.com/ajv-validator/ajv/blob/master/docs/security.md#untrusted-schemas">security considerations</a>
</li>
</references>
</qhelp>
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* @name Resources exhaustion from deep object traversal
* @description Processing user-controlled object hierarchies inefficiently can lead to denial of service.
* @kind path-problem
* @problem.severity warning
* @precision high
* @id js/resource-exhaustion-from-deep-object-traversal
* @tags security
* external/cwe/cwe-400
*/

import javascript
import DataFlow::PathGraph
import semmle.javascript.security.dataflow.DeepObjectResourceExhaustion::DeepObjectResourceExhaustion

from
Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node link,
string reason
where
cfg.hasFlowPath(source, sink) and
sink.getNode().(Sink).hasReason(link, reason)
select sink, source, sink, "Denial of service caused by processing user input from $@ with $@.",
source.getNode(), "here", link, reason
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import express from 'express';
import Ajv from 'ajv';

let ajv = new Ajv({ allErrors: true });
ajv.addSchema(require('./input-schema'), 'input');

var app = express();
app.get('/user/:id', function(req, res) {
if (!ajv.validate('input', req.body)) {
res.end(ajv.errorsText());
return;
}
// ...
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import express from 'express';
import Ajv from 'ajv';

let ajv = new Ajv({ allErrors: process.env['REST_DEBUG'] });
ajv.addSchema(require('./input-schema'), 'input');

var app = express();
app.get('/user/:id', function(req, res) {
if (!ajv.validate('input', req.body)) {
res.end(ajv.errorsText());
return;
}
// ...
});
1 change: 1 addition & 0 deletions javascript/ql/src/javascript.qll
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import semmle.javascript.InclusionTests
import semmle.javascript.JSDoc
import semmle.javascript.JSON
import semmle.javascript.JsonParsers
import semmle.javascript.JsonSchema
import semmle.javascript.JsonStringifiers
import semmle.javascript.JSX
import semmle.javascript.Lines
Expand Down
129 changes: 129 additions & 0 deletions javascript/ql/src/semmle/javascript/JsonSchema.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
/**
* Provides classes and predicates for working with JSON schema libraries.
*/

import javascript

/**
* Provides classes and predicates for working with JSON schema libraries.
*/
module JsonSchema {
/** A call that validates an input against a JSON schema. */
abstract class ValidationCall extends DataFlow::CallNode {
/** Gets the data flow node whose value is being validated. */
abstract DataFlow::Node getInput();

/** Gets the return value that indicates successful validation. */
boolean getPolarity() { result = true }
}

/** A data flow node that is used a JSON schema. */
abstract class SchemaRoot extends DataFlow::Node { }

/** An object literal with a `$schema` property indicating it is the root of a JSON schema. */
private class SchemaNodeByTag extends SchemaRoot, DataFlow::ObjectLiteralNode {
SchemaNodeByTag() {
getAPropertyWrite("$schema").getRhs().getStringValue().matches("%//json-schema.org%")
}
}

/** Gets a data flow node that is part of a JSON schema. */
private DataFlow::SourceNode getAPartOfJsonSchema(DataFlow::TypeBackTracker t) {
t.start() and
result = any(SchemaRoot n).getALocalSource()
or
result = getAPartOfJsonSchema(t.continue()).getAPropertySource()
or
exists(DataFlow::TypeBackTracker t2 | result = getAPartOfJsonSchema(t2).backtrack(t2, t))
}

/** Gets a data flow node that is part of a JSON schema. */
DataFlow::SourceNode getAPartOfJsonSchema() {
result = getAPartOfJsonSchema(DataFlow::TypeBackTracker::end())
}

/** Provides a model of the `ajv` library. */
module Ajv {
/** A method on `Ajv` that returns `this`. */
private string chainedMethod() {
result =
["addSchema", "addMetaSchema", "removeSchema", "addFormat", "addKeyword", "removeKeyword"]
}

/** An instance of `ajv`. */
class Instance extends API::InvokeNode {
Instance() { this = API::moduleImport("ajv").getAnInstantiation() }

/** Gets the data flow node holding the options passed to this `Ajv` instance. */
DataFlow::Node getOptionsArg() { result = getArgument(0) }

/** Gets an API node that refers to this object. */
API::Node ref() {
result = getReturn()
or
result = ref().getMember(chainedMethod()).getReturn()
}

/**
* Gets an API node for a function produced by `new Ajv().compile()` or similar.
*
* Note that this does not include the instance method `new Ajv().validate` as its
* signature is different.
*/
API::Node getAValidationFunction() {
result = ref().getMember(["compile", "getSchema"]).getReturn()
or
result = ref().getMember("compileAsync").getPromised()
}

/**
* Gets an API node that refers to an error produced by this Ajv instance.
*/
API::Node getAValidationError() {
exists(API::Node base | base = [ref(), getAValidationFunction()] |
result = base.getMember("errors")
or
result = base.getMember("errorsText").getReturn()
)
}
}

/** A call to the `validate` method of `ajv`. */
class AjvValidationCall extends ValidationCall {
Instance instance;
int argIndex;

AjvValidationCall() {
this = instance.ref().getMember("validate").getACall() and argIndex = 1
or
this = instance.getAValidationFunction().getACall() and argIndex = 0
}

override DataFlow::Node getInput() { result = getArgument(argIndex) }

/** Gets the argument holding additional options to the call. */
DataFlow::Node getOwnOptionsArg() { result = getArgument(argIndex + 1) }

/** Gets a data flow passed as the extra options to this validation call or to the underlying `Ajv` instance. */
DataFlow::Node getAnOptionsArg() {
result = getOwnOptionsArg()
or
result = instance.getOptionsArg()
}

/** Gets the ajv instance doing the validation. */
Instance getAjvInstance() { result = instance }
}

private class AjvSchemaNode extends SchemaRoot {
AjvSchemaNode() {
this =
any(Instance i)
.ref()
.getMember(["addSchema", "validate", "compile", "compileAsync"])
.getParameter(0)
.getARhs()
}
}
}
}
11 changes: 11 additions & 0 deletions javascript/ql/src/semmle/javascript/Regexp.qll
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,17 @@ predicate isInterpretedAsRegExp(DataFlow::Node source) {
// because `String.prototype.search` returns a number
not exists(PropAccess p | p.getBase() = mce.getEnclosingExpr())
)
or
exists(DataFlow::SourceNode schema | schema = JsonSchema::getAPartOfJsonSchema() |
source = schema.getAPropertyWrite("pattern").getRhs()
or
source =
schema
.getAPropertySource("patternProperties")
.getAPropertyWrite()
.getPropertyNameExpr()
.flow()
)
)
}

Expand Down
36 changes: 20 additions & 16 deletions javascript/ql/src/semmle/javascript/security/TaintedObject.qll
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,16 @@
import javascript
private import semmle.javascript.dataflow.InferredTypes

/** Provides classes and predicates for reasoning about deeply tainted objects. */
module TaintedObject {
private import DataFlow
import TaintedObjectCustomizations::TaintedObject

private class TaintedObjectLabel extends FlowLabel {
TaintedObjectLabel() { this = "tainted-object" }
// Materialize flow labels
private class ConcreteTaintedObjectLabel extends TaintedObjectLabel {
ConcreteTaintedObjectLabel() { this = this }
}

/**
* Gets the flow label representing a deeply tainted object.
*
* A "tainted object" is an array or object whose property values are all assumed to be tainted as well.
*
* Note that the presence of the this label generally implies the presence of the `taint` label as well.
*/
FlowLabel label() { result instanceof TaintedObjectLabel }

/**
* Holds for the flows steps that are relevant for tracking user-controlled JSON objects.
*/
Expand Down Expand Up @@ -79,11 +73,6 @@ module TaintedObject {
*/
predicate isSource(Node source, FlowLabel label) { source instanceof Source and label = label() }

/**
* A source of a user-controlled deep object.
*/
abstract class Source extends DataFlow::Node { }

/** Request input accesses as a JSON source. */
private class RequestInputAsSource extends Source {
RequestInputAsSource() { this.(HTTP::RequestInputAccess).isUserControlledObject() }
Expand Down Expand Up @@ -120,4 +109,19 @@ module TaintedObject {
label = label()
}
}

/**
* A sanitizer guard that validates an input against a JSON schema.
*/
private class JsonSchemaValidationGuard extends SanitizerGuard {
JsonSchema::ValidationCall call;

JsonSchemaValidationGuard() { this = call }

override predicate sanitizes(boolean outcome, Expr e, FlowLabel label) {
outcome = call.getPolarity() and
e = call.getInput().asExpr() and
label = label()
}
}
}
Loading