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

BREAKING CHANGE(core): limit regex to ReDoS safe patterns #484

Merged
merged 7 commits into from
Aug 15, 2022

Conversation

mttrbrts
Copy link
Member

@mttrbrts mttrbrts commented Aug 12, 2022

This PR changes the Regular Expression engine for String Validators to use re2-wasm. re2-wasm is a drop-in replacement that prohibits back-tracking. This protects us from Regular Expression Denial of Service (ReDoS) attacks on the server. Client-side code should directly validate user-provided regexes if they are being evaluated client-side.

Changes

  • Changes the implementation of the RegExp engine in StringValidator in concerto-core
  • Fixes a vulnerable regex in the metamodel! 😱
  • Refactors use of backtracking in the property name regex in the metamodel to the core runtime instead.
  • Improves the error message for Validator failures.
  • Fixes a bug with incorrect pattern code generation for JSON Schema targets.

Flags

  • This is a breaking change because existing models with regex string validators that use backreferences or lookahead assertions are no longer supported.
  • re2-wasm also uses Unicode mode by default.
  • re2-wasm is not supported in the browser, so we fallback to RegExp (i.e. the previous behaviour).
  • Added unrelated test to keep code-coverage above 98% threshold, due to unrelated change.

Author Checklist

  • Ensure you provide a DCO sign-off for your commits using the --signoff option of git commit.
  • Vital features and changes captured in unit and/or integration tests
  • Commits messages follow AP format
  • Extend the documentation, if necessary
  • Merging to master from fork:branchname

@mttrbrts mttrbrts requested review from dselman and sstone1 August 12, 2022 14:14
@mttrbrts mttrbrts changed the title BREAKING CHANGE(core): limit regex to redos safe patterns BREAKING CHANGE(core): limit regex to ReDoS safe patterns Aug 15, 2022
Copy link
Contributor

@dselman dselman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question about the JSON Schema regex - but otherwise looks good.

@mttrbrts mttrbrts merged commit 41b98bb into accordproject:master Aug 15, 2022
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 this pull request may close these issues.

2 participants