Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support configuring AJV allErrors to follow their recommended security practices #954

Closed
mdmower-csnw opened this issue Aug 30, 2024 · 0 comments · Fixed by #955
Closed

Comments

@mdmower-csnw
Copy link
Contributor

Is your feature request related to a problem? Please describe.

In AJV's security considerations documentation, they write "Do NOT use allErrors in production":

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 - in some cases using maxLength can help mitigate it, but certain regular expressions can lead to exponential validation time even with relatively short strings (see ReDoS attack).
  • patternProperties for large property names - use propertyNames to mitigate, but some regular expressions can have exponential evaluation time as well.
  • uniqueItems for large non-scalar arrays - use maxItems to mitigate

Do NOT use allErrors in production

The suggestions above to prevent slow validation would only work if you do NOT use allErrors: true in production code (using it would continue validation after validation errors).

Unfortunately, express-openapi-validator overrides whatever the user attempts to set for allErrors:

const ajv = new AjvDraft4({
...ajvOptions,
allErrors: true,
formats: formats,
});

Note: allErrors is also set to true in OpenAPISchemaValidator, but that is less concerning since it is just used for OpenAPI schema validation and not end user requests.

Describe the solution you'd like
It should be possible for developers to set allErrors: false and express-openapi-validator will respect it.

Describe alternatives you've considered
(none)

Additional context
This could help mitigate ReDOS attacks, at least to a small extent.

mdmower-csnw added a commit to CSNW/express-openapi-validator that referenced this issue Aug 30, 2024
AJV recommends setting option `allErrors` to `false` in production.
pdate `createAjv()` to respect the user's setting. Avoid introducing a
breaking change by defaulting to `true` when not defined by the user.

Add tests:
1. Make sure `AjvOptions` sets the value appropriately based on whether
   the end user defined `allErrors` or not.
2. When validating requests, make sure the number of errors reported
   (when multiple occur) is 1 when `allErrors` is `false`.

The `allErrors` configuration for OpenAPISchemaValidator is not changed
by this commit since that validation is for trusted content.

Fixes cdimascio#954
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant