-
-
Notifications
You must be signed in to change notification settings - Fork 109
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
Several issues with using this plugin #22
Comments
1 - This is a known issue unfortunately. If you look at the source you can see that we are only looking for method calls. If somebody wants to track those references we would accept a pull request. 2 - We could probably improve the documentation here. To clarify for you it's simply pointing out that it could be user input in one of those calls (a variable vs a literal string), which could or could not be bad depending on the context. 3- We process these using a module called safe-regex which has known false positives AND false negatives. We use this rule to trigger re's that may need further in 4- Yup. this is a pretty overwhelming one. We have used it to find issues but it's most certainly a needle in a haystack type assessment when this rule is enabled. Thanks for your feedback. We use these rules to audit code and identify issues quickly for human review. We find them useful. I'm sorry they frustrate you. We would love them to be better and unless we find significant time to invest or get pull requests to do more state tracking with the underlying ast we can't do much more than what we have. |
@rossPatton about safe-regex, it may also report issues due to a potential complexity detected. |
I think 1 and 2 can probably be fixed in #109. |
security/detect-non-literal-fs-filename
rule reportsfs.${whatever}
regardless of whether that's what i'm using or not. For example, i use jsonFile a lot, so an error withjsonFile.readFile
gets reported asfs.readFile
, when there is no reference to fs on that page. Sure, the error probably still applies, but it's confusing when the error output doesn't line up with the actual codeIt's unclear, even with the linked resources, what exactly needs to be done to resolve an issue, or even what the issue is. With the above error, I was not at all sure what the problem really was or how to resolve it. For reference, that file (and just about all instances of that error on my code) are just using paths like this:
${process.cwd()}/path/to/json
. I have a hard time seeing how this is exploitable?I'm getting several unsafe regex errors. Most of them were common regexes (verify zip codes, stuff like that). So as recommended, I used the OWASP link to replace them with their safe variants. Even after replacing them, the error still persists
detect-object-injection
. As this test is currently designed, it's completely overwhelmingI really want to use this plugin to easily catch minor security issues, but as is, i can't imagine it doing anything more than frustrate people
The text was updated successfully, but these errors were encountered: