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

[Bug]: Inefficient regex in the no-unknown-property rule #3666

Closed
2 tasks done
SCH227 opened this issue Dec 14, 2023 · 5 comments
Closed
2 tasks done

[Bug]: Inefficient regex in the no-unknown-property rule #3666

SCH227 opened this issue Dec 14, 2023 · 5 comments

Comments

@SCH227
Copy link

SCH227 commented Dec 14, 2023

Is there an existing issue for this?

  • I have searched the existing issues and my issue is unique
  • My issue appears in the command-line and not only in the text editor

Description Overview

The regex in charge of parsing data attributes in the no-unknown-property rule is vulnerable to catastrophic backtracking:
https://github.com/jsx-eslint/eslint-plugin-react/blob/master/lib/rules/no-unknown-property.js#L431

Here's an example payload:
https://regex101.com/r/GvfmhG/1

As a result, the function isValidDataAttribute(name) is affected.

According to the project maintainers contacted via the security channel, there aren't security concerns given how eslint-plugin-react is used.

Possible Fix:
The root cause of the exponential complexity in isValidDataAttribute() regex seems to be the nested quantifier. In my tests, the following regex avoids that while retaining the same matching capability as the old one: ^data(-?[-[^:]]*)$
Note that both regexes (the old and the proposed one) match "data" or "data-", which are not valid according to the HTML5 specification.

Expected Behavior

Process executing not in exponential time.

eslint-plugin-react version

v7.33.2

eslint version

v8.54.0

node version

v18.13.0

@SCH227 SCH227 added the bug label Dec 14, 2023
@ljharb
Copy link
Member

ljharb commented Dec 14, 2023

Unfortunately using that regex causes 16 test failures.

@SCH227
Copy link
Author

SCH227 commented Dec 14, 2023

Sorry, the proposed regex fix was miswritten. This should work ok: ^data(-?[^:]*)$

@ljharb ljharb closed this as completed in 96af875 Dec 14, 2023
@ljharb
Copy link
Member

ljharb commented Dec 14, 2023

Thanks!

@jeremydrichardson
Copy link

You mention:

Note that both regexes (the old and the proposed one) match "data" or "data-", which are not valid according to the HTML5 specification.

Did you ever find an explanation as to why that is? That means it's going to incorrectly flag props that start with data (eg. dataLayer, dataAmount, etc...)

@ljharb
Copy link
Member

ljharb commented Mar 25, 2024

@jeremydrichardson if that’s the case, please file a new issue and we can fix that too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants