diff --git a/README.md b/README.md index 1e394e7..a5acea4 100644 --- a/README.md +++ b/README.md @@ -608,16 +608,19 @@ See [Options](#options). ## Security considerations +JSON Schema, if properly used, can replace data sanitisation. It doesn't replace other API security considerations. It also introduces additional security aspects to consider. + + ##### Untrusted schemas Ajv treats JSON schemas as trusted as your application code. This security model is based on the most common use case, when the schemas are static and bundled together with the application. -If your schemas are received from untrusted sources (or generated from untrusted data) there may be several scenarios you may want to prevent: +If your schemas are received from untrusted sources (or generated from untrusted data) there are several scenarios you need to prevent: - compiling schemas can cause stack overflow (if they are too deep) - compiling schemas can be slow (e.g. [#557](https://github.com/epoberezkin/ajv/issues/557)) - validating certain data can be slow -It is difficult to predict all the scenarios, but at the very least it is recommended to limit the size of untrusted JSON Schemas (e.g. as JSON string length) and the maximum schema object depth (that can be high for relatively small JSON strings). Even that would not prevent slow regular expressions in schemas. +It is difficult to predict all the scenarios, but at the very least it may help to limit the size of untrusted schemas (e.g. limit JSON string length) and also the maximum schema object depth (that can be high for relatively small JSON strings). You also may want to mitigate slow regular expressions in `pattern` and `patternProperties` keywords. Regardless the measures you take, using untrusted schemas increases security risks. @@ -626,12 +629,12 @@ Regardless the measures you take, using untrusted schemas increases security ris Ajv does not support schemas and validated data that have circular references in objects. See [issue #802](https://github.com/epoberezkin/ajv/issues/802). -An attempt to compile such schemas or validate such data would cause stack overflow (or will not complete in case of asynchronous validation). Untrusted data can lead to circular references, depending on the parser you use. +An attempt to compile such schemas or validate such data would cause stack overflow (or will not complete in case of asynchronous validation). Depending on the parser you use, untrusted data can lead to circular references. ##### Security risks of trusted schemas -Some keywords in JSON Schemas can lead to very slow validation for certain data. These keywords include (but, most likely, not limited to): +Some keywords in JSON Schemas can lead to very slow validation for certain data. These keywords include (but may be not limited to): - `pattern` and `format` for large strings - use `maxLength` to mitigate - `uniqueItems` for large non-scalar arrays - use `maxItems` to mitigate @@ -651,7 +654,7 @@ const schema2 = {format: 'email', maxLength: 256}; isSchemaSecure(schema2); // true ``` -__Please note__: even following all these recommendation is not a guarantee that validation of untrusted data is absolutely safe - it can still lead to some undesirable situations. +__Please note__: even following all these recommendation is not a guarantee that validation of untrusted data is absolutely safe - it can still lead to some undesirable results. ## Filtering data diff --git a/lib/refs/json-schema-secure.json b/lib/refs/json-schema-secure.json index d6a8e4c..fea4d04 100644 --- a/lib/refs/json-schema-secure.json +++ b/lib/refs/json-schema-secure.json @@ -31,16 +31,20 @@ "required": ["uniqueItems"], "properties": { "uniqueItems": {"const": true}, - "type": { - "anyOf": [ - { - "enum": ["object", "array"] - }, - { - "type": "array", - "contains": {"enum": ["object", "array"]} + "items": { + "properties": { + "type": { + "anyOf": [ + { + "enum": ["object", "array"] + }, + { + "type": "array", + "contains": {"enum": ["object", "array"]} + } + ] } - ] + } } } }, diff --git a/spec/security.spec.js b/spec/security.spec.js new file mode 100644 index 0000000..182e8b8 --- /dev/null +++ b/spec/security.spec.js @@ -0,0 +1,27 @@ +'use strict'; + +var jsonSchemaTest = require('json-schema-test') + , getAjvInstances = require('./ajv_instances') + , options = require('./ajv_options') + , suite = require('./browser_test_suite') + , after = require('./after_test'); + +var instances = getAjvInstances(options, { + schemas: [require('../lib/refs/json-schema-secure.json')] +}); + + +jsonSchemaTest(instances, { + description: 'Secure schemas tests of ' + instances.length + ' ajv instances with different options', + suites: { + 'security': typeof window == 'object' + ? suite(require('./security/{**/,}*.json', {mode: 'list'})) + : './security/{**/,}*.json' + }, + assert: require('./chai').assert, + afterError: after.error, + afterEach: after.each, + cwd: __dirname, + hideFolder: 'security/', + timeout: 90000 +}); diff --git a/spec/security/array.json b/spec/security/array.json new file mode 100644 index 0000000..b089a49 --- /dev/null +++ b/spec/security/array.json @@ -0,0 +1,104 @@ +[ + { + "description": "uniqueItems without type keyword should be used together with maxItems", + "schema": {"$ref": "https://raw.githubusercontent.com/epoberezkin/ajv/master/lib/refs/json-schema-secure.json#"}, + "tests": [ + { + "description": "uniqueItems keyword used without maxItems is unsafe", + "data": { + "uniqueItems": true + }, + "valid": false + }, + { + "description": "uniqueItems keyword used with maxItems is safe", + "data": { + "uniqueItems": true, + "maxItems": "10" + }, + "valid": true + }, + { + "description": "uniqueItems: false is ignored (and safe)", + "data": { + "uniqueItems": false + }, + "valid": true + } + ] + }, + { + "description": "uniqueItems with scalar type(s) is safe to use without maxItems", + "schema": {"$ref": "https://raw.githubusercontent.com/epoberezkin/ajv/master/lib/refs/json-schema-secure.json#"}, + "tests": [ + { + "description": "uniqueItems keyword with a single scalar type is safe", + "data": { + "uniqueItems": true, + "items": { + "type": "number" + } + }, + "valid": true + }, + { + "description": "uniqueItems keyword with multiple scalar types is safe", + "data": { + "uniqueItems": true, + "items": { + "type": ["number", "string"] + } + }, + "valid": true + } + ] + }, + { + "description": "uniqueItems with compound type(s) should be used together with maxItems", + "schema": {"$ref": "https://raw.githubusercontent.com/epoberezkin/ajv/master/lib/refs/json-schema-secure.json#"}, + "tests": [ + { + "description": "uniqueItems keyword with a single compound type and without maxItems is unsafe", + "data": { + "uniqueItems": true, + "items": { + "type": "object" + } + }, + "valid": false + }, + { + "description": "uniqueItems keyword with a single compound type and with maxItems is safe", + "data": { + "uniqueItems": true, + "maxItems": "10", + "items": { + "type": "object" + } + }, + "valid": true + }, + { + "description": "uniqueItems keyword with multiple types including compound type and without maxItems is unsafe", + "data": { + "uniqueItems": true, + "items": { + "type": ["array","number"] + } + }, + "valid": false + }, + { + "description": "uniqueItems keyword with multiple types including compound type and with maxItems is safe", + "data": { + "uniqueItems": true, + "maxItems": "10", + "items": { + "type": ["array","number"] + } + }, + "valid": true + } + ] + } +] diff --git a/spec/security/object.json b/spec/security/object.json new file mode 100644 index 0000000..e8efede --- /dev/null +++ b/spec/security/object.json @@ -0,0 +1,29 @@ +[ + { + "description": "patternProperties keyword should be used together with propertyNames", + "schema": { "$ref": "https://raw.githubusercontent.com/epoberezkin/ajv/master/lib/refs/json-schema-secure.json#" }, + "tests": [ + { + "description": "patternProperties keyword used without propertyNames is unsafe", + "data": { + "patternProperties": { + ".*": {} + } + }, + "valid": false + }, + { + "description": "patternProperties keyword used with propertyNames is safe", + "data": { + "patternProperties": { + ".*": {} + }, + "propertyNames": { + "maxLength": "256" + } + }, + "valid": true + } + ] + } +] diff --git a/spec/security/string.json b/spec/security/string.json new file mode 100644 index 0000000..6aa4f86 --- /dev/null +++ b/spec/security/string.json @@ -0,0 +1,44 @@ +[ + { + "description": "pattern keyword should be used together with maxLength", + "schema": { "$ref": "https://raw.githubusercontent.com/epoberezkin/ajv/master/lib/refs/json-schema-secure.json#" }, + "tests": [ + { + "description": "pattern keyword used without maxLength is unsafe", + "data": { + "pattern": ".*" + }, + "valid": false + }, + { + "description": "pattern keyword used with maxLength is safe", + "data": { + "pattern": ".*", + "maxLength": "256" + }, + "valid": true + } + ] + }, + { + "description": "format keyword should be used together with maxLength", + "schema": { "$ref": "https://raw.githubusercontent.com/epoberezkin/ajv/master/lib/refs/json-schema-secure.json#" }, + "tests": [ + { + "description": "format keyword used without maxLength is unsafe", + "data": { + "format": "email" + }, + "valid": false + }, + { + "description": "format keyword used with maxLength is safe", + "data": { + "format": "email", + "maxLength": "256" + }, + "valid": true + } + ] + } +]