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 JWT pattern #179
Fix JWT pattern #179
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change referring to the #178 issue, but note that it is not a JWT issue on itself
Same issue can happen with all short patterns
So it would be more correct to apply fix to all pattern based regular expressions
credsweeper/rules/config.yaml
Outdated
@@ -162,7 +162,7 @@ | |||
severity: medium | |||
type: pattern | |||
values: | |||
- (?P<value>eyJ[A-Za-z0-9-_=]+(\.[A-Za-z0-9-_.+\/=]+)?) | |||
- (?<![0-9a-zA-Z_-])(?P<value>eyJ[A-Za-z0-9-_=]+\.eyJ[A-Za-z0-9-_=]+(\.[A-Za-z0-9-_=]+)?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JWT with a single eyJ
entry might still be a valid credential
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?<![0-9a-zA-Z_-])
probably should not be in JWT regular expression itself
We can concatenate it to ALL pattern based expressions, so it would fix all other short expressions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'''(?<![0-9a-zA-Z_-])''' removes case when eyJ is only a substring in long base64 case. even (?<![0-9a-zA-Z])
might reduce suspicious strings.
fixes False positive for short regular expressions #178