-
Notifications
You must be signed in to change notification settings - Fork 2k
Improve actions/ql/src/Security/CWE-829/UntrustedCheckoutX queries #21715
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
base: main
Are you sure you want to change the base?
Changes from all commits
1233d81
a342efc
c9e5dbd
589e1e5
ed4e2bc
81532c7
4fd0222
b0bc0fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| --- | ||
| category: minorAnalysis | ||
| --- | ||
| * Altered 2 patterns in the `poisonable_steps` modelling. Extra sinks are detected in the following cases: scripts executed via python modules and `go run` in directories are detected as potential mechanisms of injection. For the go execution pattern, the pattern is updated to now ignore flags that occur between go and the specific command. This change may lead to more results being detected by any queries that use that library. | ||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||
| /** | ||||||
| * @name Checkout of untrusted code in trusted context | ||||||
| * @name Checkout of untrusted code in privileged context without privileged context use | ||||||
|
||||||
| * @name Checkout of untrusted code in privileged context without privileged context use | |
| * @name Checkout of untrusted code in a privileged workflow |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| category: majorAnalysis | ||
| --- | ||
| * Fixed help file descriptions for queries: `actions/untrusted-checkout/critical`, `actions/untrusted-checkout/high`, `actions/untrusted-checkout/medium`. Previously the messages were unclear as to why and how the vulnerabilities could occur. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need a change note for qhelp files. It isn't harmful though, so if a customer particularly wants it then we can keep it. But it definitely shouldn't be in the |
||
| * Adjusted `actions/untrusted-checkout/critical` to align more with other untrusted resource queries, where the alert location is the location where the artifact is obtained from (the checkout point). This aligns with the other 2 related queries. This will cause the same alerts to re-open for closed alerts of this query. | ||
| * Adjusted the name of `actions/untrusted-checkout/high` to more clearly describe which parts of the scenario are in a privileged context. This will cause the same alerts to re-open for closed alerts of this query. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it definitely correct that this will cause alerts to reopen? I'd have thought that the query id is used for matching alerts up - I'm pretty sure I've changed the query name before and not had a problem. I've asked internally to be sure.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, this should have the category |
||
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.
Please specify the queries. We want a user who sees extra results for query X and then looks at the change note to see why to immediately find the answer.