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

Fix Catastophic Backtracking for Indirect Reference #509

Merged
merged 3 commits into from
Feb 2, 2022

Conversation

jpdakran
Copy link
Member

@jpdakran jpdakran commented Feb 1, 2022

Problem

  • I've noticed that there is a hang when running detect-secrets.
  • This is due to the regex that the Indirect-Reference filter utilizes.
  • The problem here is catastrophic backtracking. This occurs when we are trying to match a line that is typically more than 25k characters against the regex.

Solution

  • We have a few options here
  1. Add conditions on when to run the regex for a line - 1k characters or less (or some agreed up length)
  2. Update the regex
  • I resorted to option 1. The intention of the heuristic should be for short and simple lines.

@jpdakran jpdakran merged commit af04c48 into master Feb 2, 2022
@@ -165,6 +165,8 @@ def is_indirect_reference(line: str) -> bool:

secret = request.headers['apikey']
"""
if len(line) > 1000:
Copy link
Contributor

Choose a reason for hiding this comment

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

@jpdakran : we should document design rationales in code to assist future developers in understanding the codebase.

Copy link
Member Author

Choose a reason for hiding this comment

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

Understood. Please see #516 for update.

@jpdakran jpdakran deleted the jdakran_fix_catastrophic_backtracking branch February 11, 2022 21:19
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.

None yet

3 participants