-
Notifications
You must be signed in to change notification settings - Fork 1.8k
github actions queries #5350
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
github actions queries #5350
Conversation
javascript/ql/src/experimental/Security/CWE-829/expression_injection.ql
Outdated
Show resolved
Hide resolved
Initially developed by @adityasharad I have made few improvements. |
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.
Nice work.
I think it should be possible to make these queries more precise.
E.g. looking for a build step that executes code in pull_request_target.ql
.
But that's for another time, the queries look good for experimental as they are.
The autoformatter is failing on pull_request_target.ql
.
(I can probably fix that for you if you want).
javascript/ql/src/experimental/Security/CWE-829/expression_injection.ql
Outdated
Show resolved
Hide resolved
javascript/ql/src/experimental/Security/CWE-829/expression_injection.ql
Outdated
Show resolved
Hide resolved
javascript/ql/src/experimental/Security/CWE-829/examples/comment_pr.yml
Outdated
Show resolved
Hide resolved
javascript/ql/src/experimental/Security/CWE-829/pull_request_target.ql
Outdated
Show resolved
Hide resolved
javascript/ql/src/experimental/Security/CWE-829/pull_request_target.ql
Outdated
Show resolved
Hide resolved
Co-authored-by: Erik Krogh Kristensen <erik-krogh@github.com>
Tried the format command, but it doesn't change anything in the document, please check. |
Co-authored-by: Erik Krogh Kristensen <erik-krogh@github.com>
I see it fails on a test complaining about regex. Interestingly it works fine locally when I run jus these two tests. Any idea how to fix? |
Co-authored-by: Erik Krogh Kristensen <erik-krogh@github.com>
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.
I think we are good to go for merging into experimental.
Could you give me a ping when you get CVEs from these queries (I didn't find CVEs in your published advisories, but I also didn't look very hard).
Because such CVEs could be helpful for improving the queries, and from there get the queries into the default suite.
We decided not to request CVEs because these queries find vulnerabilities in CI/CD configuration, but not in a final product. Since there is no immediate action a consumer would need to take, like upgrade to the new version, it made no sense to request CVEs. Theses articles may be useful to improve the queries in the future: If you want to examine real world occurrences look here https://securitylab.github.com/advisories At the moment there are ~100 workflow related reports. |
No description provided.