-
Notifications
You must be signed in to change notification settings - Fork 2
Remove dependencies on isSink
in the taint tracking configurations of the default queries
#180
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
Conversation
isSink
in the taint tracking configurations of the default queries
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.
Thanks @jeongsoolee09!
As noted in the review comments, I think it's both fine and desirable for us to extend the default configurations for the queries we've implemented, so long as we ensure isSink
specifies only SAP specific sinks not reported by the standard library. Then we're guaranteed to avoid duplication.
...ript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CAPLogInjectionQuery.qll
Outdated
Show resolved
Hide resolved
javascript/frameworks/ui5/lib/advanced_security/javascript/frameworks/ui5/UI5XssQuery.qll
Outdated
Show resolved
Hide resolved
...t/frameworks/xsjs/lib/advanced_security/javascript/frameworks/xsjs/XSJSReflectedXssQuery.qll
Outdated
Show resolved
Hide resolved
Thanks @lcartey! I agree with the CAP log injection and XSJS Reflected XSS queries needing to extend the default configurations, but will deleting the The only thing that line does is to make queries with |
Yep, totally agree - I made the configurations extend the default ones again, but retained the deletion of |
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.
Looks good, thanks!
Remove dependencies on
isSink
of the standard library configurations in the custom taint tracking configurations. This will hopefully remove the duplicate XSS alerts raised by the default queries. These are done by:super.isSink(...)
in the custom configurations to make the kind of vulnerabilities sink dependent.UI5Xss
query.isSource
definitions.