Skip to content

Commit 1b30847

Browse files
authored
Merge branch 'main' into rust-rusqlite
2 parents 3e38867 + 9eeae71 commit 1b30847

File tree

70 files changed

+1575
-144
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

70 files changed

+1575
-144
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: feature
3+
---
4+
* The "Unpinned tag for a non-immutable Action in workflow" query (`actions/unpinned-tag`) now supports expanding the trusted action owner list using data extensions (`extensible: trustedActionsOwnerDataModel`). If you trust an Action publisher, you can include the owner name/organization in a model pack to add it to the allow list for this query. This addition will prevent security alerts when using unpinned tags for Actions published by that owner. For more information on creating a model pack, see [Creating a CodeQL Model Pack](https://docs.github.com/en/code-security/codeql-cli/using-the-advanced-functionality-of-the-codeql-cli/creating-and-working-with-codeql-packs#creating-a-codeql-model-pack).

actions/ql/lib/codeql/actions/config/Config.qll

+9
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,15 @@ predicate vulnerableActionsDataModel(
126126
*/
127127
predicate immutableActionsDataModel(string action) { Extensions::immutableActionsDataModel(action) }
128128

129+
/**
130+
* MaD models for trusted actions owners
131+
* Fields:
132+
* - owner: owner name
133+
*/
134+
predicate trustedActionsOwnerDataModel(string owner) {
135+
Extensions::trustedActionsOwnerDataModel(owner)
136+
}
137+
129138
/**
130139
* MaD models for untrusted git commands
131140
* Fields:

actions/ql/lib/codeql/actions/config/ConfigExtensions.qll

+5
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ extensible predicate vulnerableActionsDataModel(
6363
*/
6464
extensible predicate immutableActionsDataModel(string action);
6565

66+
/**
67+
* Holds for trusted Actions owners.
68+
*/
69+
extensible predicate trustedActionsOwnerDataModel(string owner);
70+
6671
/**
6772
* Holds for git commands that may introduce untrusted data when called on an attacker controlled branch.
6873
*/
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/actions-all
4+
extensible: trustedActionsOwnerDataModel
5+
data:
6+
- ["actions"]
7+
- ["github"]
8+
- ["advanced-security"]

actions/ql/src/Security/CWE-077/EnvPathInjectionMedium.ql

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
* @name PATH Enviroment Variable built from user-controlled sources
33
* @description Building the PATH environment variable from user-controlled sources may alter the execution of following system commands
44
* @kind path-problem
5-
* @problem.severity warning
5+
* @problem.severity error
66
* @security-severity 5.0
7-
* @precision high
7+
* @precision medium
88
* @id actions/envpath-injection/medium
99
* @tags actions
1010
* security

actions/ql/src/Security/CWE-077/EnvVarInjectionMedium.ql

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22
* @name Enviroment Variable built from user-controlled sources
33
* @description Building an environment variable from user-controlled sources may alter the execution of following system commands
44
* @kind path-problem
5-
* @problem.severity warning
5+
* @problem.severity error
66
* @security-severity 5.0
7-
* @precision high
7+
* @precision medium
88
* @id actions/envvar-injection/medium
99
* @tags actions
1010
* security

actions/ql/src/Security/CWE-275/MissingActionsPermissions.ql

+2-1
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,12 @@
33
* @description Workflows should contain permissions to provide a clear understanding has permissions to run the workflow.
44
* @kind problem
55
* @security-severity 5.0
6-
* @problem.severity recommendation
6+
* @problem.severity warning
77
* @precision high
88
* @id actions/missing-workflow-permissions
99
* @tags actions
1010
* maintainability
11+
* security
1112
* external/cwe/cwe-275
1213
*/
1314

actions/ql/src/Security/CWE-312/ExcessiveSecretsExposure.ql

+2-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
* @name Excessive Secrets Exposure
33
* @description All organization and repository secrets are passed to the workflow runner.
44
* @kind problem
5-
* @problem.severity recommendation
5+
* @precision high
6+
* @problem.severity warning
67
* @id actions/excessive-secrets-exposure
78
* @tags actions
89
* security

actions/ql/src/Security/CWE-829/ArtifactPoisoningMedium.ql

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
* @name Artifact poisoning
33
* @description An attacker may be able to poison the workflow's artifacts and influence on consequent steps.
44
* @kind path-problem
5-
* @problem.severity warning
6-
* @precision high
5+
* @problem.severity error
6+
* @precision medium
77
* @security-severity 5.0
88
* @id actions/artifact-poisoning/medium
99
* @tags actions

actions/ql/src/Security/CWE-829/UnpinnedActionsTag.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,4 @@ Pinning an action to a full length commit SHA is currently the only way to use a
2424
2525
## References
2626
27-
- [Using third-party actions](https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions)
27+
- [Using third-party actions](https://docs.github.com/en/actions/security-for-github-actions/security-guides/security-hardening-for-github-actions#using-third-party-actions)

actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql

+11-10
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
* @description Using a tag for a non-immutable Action that is not pinned to a commit can lead to executing an untrusted Action through a supply chain attack.
44
* @kind problem
55
* @security-severity 5.0
6-
* @problem.severity recommendation
7-
* @precision high
6+
* @problem.severity warning
7+
* @precision medium
88
* @id actions/unpinned-tag
99
* @tags security
1010
* actions
@@ -17,24 +17,25 @@ import codeql.actions.security.UseOfUnversionedImmutableAction
1717
bindingset[version]
1818
private predicate isPinnedCommit(string version) { version.regexpMatch("^[A-Fa-f0-9]{40}$") }
1919

20-
bindingset[repo]
21-
private predicate isTrustedOrg(string repo) {
22-
repo.matches(["actions", "github", "advanced-security"] + "/%")
20+
bindingset[nwo]
21+
private predicate isTrustedOwner(string nwo) {
22+
// Gets the segment before the first '/' in the name with owner(nwo) string
23+
trustedActionsOwnerDataModel(nwo.substring(0, nwo.indexOf("/")))
2324
}
2425

25-
from UsesStep uses, string repo, string version, Workflow workflow, string name
26+
from UsesStep uses, string nwo, string version, Workflow workflow, string name
2627
where
27-
uses.getCallee() = repo and
28+
uses.getCallee() = nwo and
2829
uses.getEnclosingWorkflow() = workflow and
2930
(
3031
workflow.getName() = name
3132
or
3233
not exists(workflow.getName()) and workflow.getLocation().getFile().getBaseName() = name
3334
) and
3435
uses.getVersion() = version and
35-
not isTrustedOrg(repo) and
36+
not isTrustedOwner(nwo) and
3637
not isPinnedCommit(version) and
37-
not isImmutableAction(uses, repo)
38+
not isImmutableAction(uses, nwo)
3839
select uses.getCalleeNode(),
39-
"Unpinned 3rd party Action '" + name + "' step $@ uses '" + repo + "' with ref '" + version +
40+
"Unpinned 3rd party Action '" + name + "' step $@ uses '" + nwo + "' with ref '" + version +
4041
"', not a pinned commit hash", uses, uses.toString()
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
category: breaking
3+
---
4+
* The following queries have been removed from the `code-scanning` and `security-extended` suites.
5+
Any existing alerts for these queries will be closed automatically.
6+
* `actions/if-expression-always-true/critical`
7+
* `actions/if-expression-always-true/high`
8+
* `actions/unnecessary-use-of-advanced-config`
9+
10+
* The following query has been moved from the `code-scanning` suite to the `security-extended`
11+
suite. Any existing alerts for this query will be closed automatically unless the analysis is
12+
configured to use the `security-extended` suite.
13+
* `actions/unpinned-tag`
14+
* The following queries have been added to the `security-extended` suite.
15+
* `actions/unversioned-immutable-action`
16+
* `actions/envpath-injection/medium`
17+
* `actions/envvar-injection/medium`
18+
* `actions/code-injection/medium`
19+
* `actions/artifact-poisoning/medium`
20+
* `actions/untrusted-checkout/medium`
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,4 @@
11
- description: Standard Code Scanning queries for GitHub Actions
2-
- queries: '.'
3-
- include:
4-
problem.severity:
5-
- error
6-
- recommendation
7-
- exclude:
8-
tags contain:
9-
- experimental
10-
- debug
11-
- internal
2+
- queries: .
3+
- apply: code-scanning-selectors.yml
4+
from: codeql/suite-helpers
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,4 @@
11
- description: Security-extended queries for GitHub Actions
2-
- import: codeql-suites/actions-code-scanning.qls
2+
- queries: .
3+
- apply: security-extended-selectors.yml
4+
from: codeql/suite-helpers
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Security/CWE-074/OutputClobberingHigh.ql
1+
experimental/Security/CWE-074/OutputClobberingHigh.ql
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Security/CWE-078/CommandInjectionCritical.ql
1+
experimental/Security/CWE-078/CommandInjectionCritical.ql
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Security/CWE-078/CommandInjectionMedium.ql
1+
experimental/Security/CWE-078/CommandInjectionMedium.ql
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Security/CWE-088/ArgumentInjectionCritical.ql
1+
experimental/Security/CWE-088/ArgumentInjectionCritical.ql
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Security/CWE-088/ArgumentInjectionMedium.ql
1+
experimental/Security/CWE-088/ArgumentInjectionMedium.ql
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
Security/CWE-200/SecretExfiltration.ql
1+
experimental/Security/CWE-200/SecretExfiltration.ql
22

Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
Security/CWE-284/CodeExecutionOnSelfHostedRunner.ql
1+
experimental/Security/CWE-284/CodeExecutionOnSelfHostedRunner.ql
22

Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
Security/CWE-829/ArtifactPoisoningPathTraversal.ql
1+
experimental/Security/CWE-829/ArtifactPoisoningPathTraversal.ql
22

Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Security/CWE-918/RequestForgery.ql
1+
experimental/Security/CWE-918/RequestForgery.ql
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Blazor `[Parameter]` fields bound to a variable from the route specified in the `@page` directive are now modeled as remote flow sources.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
/** Provides classes for working with `Microsoft.AspNetCore.Components` */
2+
3+
import csharp
4+
import semmle.code.csharp.frameworks.Microsoft
5+
import semmle.code.csharp.frameworks.microsoft.AspNetCore
6+
7+
/** The `Microsoft.AspNetCore.Components` namespace */
8+
class MicrosoftAspNetCoreComponentsNamespace extends Namespace {
9+
MicrosoftAspNetCoreComponentsNamespace() {
10+
this.getParentNamespace() instanceof MicrosoftAspNetCoreNamespace and
11+
this.hasName("Components")
12+
}
13+
}
14+
15+
/**
16+
* A class in the `Microsoft.AspNetCore.Components` namespace.
17+
*/
18+
private class MicrosoftAspNetCoreComponentsClass extends Class {
19+
MicrosoftAspNetCoreComponentsClass() {
20+
this.getNamespace() instanceof MicrosoftAspNetCoreComponentsNamespace
21+
}
22+
}
23+
24+
/** The `Microsoft.AspNetCore.Components.CascadingParameterAttributeBase` class. */
25+
class MicrosoftAspNetCoreComponentsCascadingParameterAttributeBaseClass extends MicrosoftAspNetCoreComponentsClass
26+
{
27+
MicrosoftAspNetCoreComponentsCascadingParameterAttributeBaseClass() {
28+
this.hasName("CascadingParameterAttributeBase")
29+
}
30+
}
31+
32+
/** The `Microsoft.AspNetCore.Components.ComponentBase` class. */
33+
class MicrosoftAspNetCoreComponentsComponentBaseClass extends MicrosoftAspNetCoreComponentsClass {
34+
MicrosoftAspNetCoreComponentsComponentBaseClass() { this.hasName("ComponentBase") }
35+
}
36+
37+
/** The `Microsoft.AspNetCore.Components.IComponent` interface. */
38+
class MicrosoftAspNetCoreComponentsIComponentInterface extends Interface {
39+
MicrosoftAspNetCoreComponentsIComponentInterface() {
40+
this.getNamespace() instanceof MicrosoftAspNetCoreComponentsNamespace and
41+
this.hasName("IComponent")
42+
}
43+
}
44+
45+
/** The `Microsoft.AspNetCore.Components.RouteAttribute` attribute. */
46+
private class MicrosoftAspNetCoreComponentsRouteAttribute extends Attribute {
47+
MicrosoftAspNetCoreComponentsRouteAttribute() {
48+
this.getType().getNamespace() instanceof MicrosoftAspNetCoreComponentsNamespace and
49+
this.getType().hasName("RouteAttribute")
50+
}
51+
}
52+
53+
/** The `Microsoft.AspNetCore.Components.ParameterAttribute` attribute. */
54+
private class MicrosoftAspNetCoreComponentsParameterAttribute extends Attribute {
55+
MicrosoftAspNetCoreComponentsParameterAttribute() {
56+
this.getType().getNamespace() instanceof MicrosoftAspNetCoreComponentsNamespace and
57+
this.getType().hasName("ParameterAttribute")
58+
}
59+
}
60+
61+
/** An ASP.NET Core (Blazor) component. */
62+
class MicrosoftAspNetCoreComponentsComponent extends Class {
63+
MicrosoftAspNetCoreComponentsComponent() {
64+
this.getABaseType+() instanceof MicrosoftAspNetCoreComponentsComponentBaseClass or
65+
this.getABaseType+() instanceof MicrosoftAspNetCoreComponentsIComponentInterface
66+
}
67+
68+
/** Gets a property whose value cascades down the component hierarchy. */
69+
Property getACascadingParameterProperty() {
70+
result = this.getAProperty() and
71+
result.getAnAttribute().getType().getBaseClass() instanceof
72+
MicrosoftAspNetCoreComponentsCascadingParameterAttributeBaseClass
73+
}
74+
75+
/** Gets the url for the route from the `Microsoft.AspNetCore.Components.RouteAttribute` of the component. */
76+
private string getRouteAttributeUrl() {
77+
exists(MicrosoftAspNetCoreComponentsRouteAttribute a | a = this.getAnAttribute() |
78+
result = a.getArgument(0).getValue()
79+
)
80+
}
81+
82+
/**
83+
* Gets a route parameter from the `Microsoft.AspNetCore.Components.RouteAttribute` of the component.
84+
*
85+
* A route parameter is defined in the URL by wrapping its name in a pair of { braces } when adding a component's @page declaration.
86+
* There are various extensions that can be added next to the parameter name, such as `:int` or `?` to make the parameter optional.
87+
* Optionally, the parameter name can start with a `*` to make it a catch-all parameter.
88+
*
89+
* An example of a route parameter is `@page "/counter/{id:int}/{other?}/{*rest}"`, from this we're getting the `id`, `other` and `rest` parameters.
90+
*/
91+
pragma[nomagic]
92+
private string getARouteParameter() {
93+
exists(string s |
94+
s = this.getRouteAttributeUrl().splitAt("{").regexpCapture("\\*?([^:?}]+)[:?}](.*)", 1) and
95+
result = s.toLowerCase()
96+
)
97+
}
98+
99+
/** Gets a property attributed with `[Parameter]` attribute. */
100+
pragma[nomagic]
101+
private Property getAParameterProperty(string name) {
102+
result = this.getAProperty() and
103+
result.getAnAttribute() instanceof MicrosoftAspNetCoreComponentsParameterAttribute and
104+
name = result.getName().toLowerCase()
105+
}
106+
107+
/** Gets a property whose value is populated from route parameters. */
108+
Property getARouteParameterProperty() {
109+
exists(string name | name = this.getARouteParameter() |
110+
result = this.getAParameterProperty(name)
111+
)
112+
}
113+
}
114+
115+
private module Sources {
116+
private import semmle.code.csharp.security.dataflow.flowsources.Remote
117+
118+
/**
119+
* A property with a `[Parameter]` attribute in an ASP.NET Core component which
120+
* is populated from a route parameter.
121+
*/
122+
private class AspNetCoreComponentRouteParameterFlowSource extends AspNetRemoteFlowSource,
123+
DataFlow::ExprNode
124+
{
125+
AspNetCoreComponentRouteParameterFlowSource() {
126+
exists(MicrosoftAspNetCoreComponentsComponent c, Property p |
127+
p = c.getARouteParameterProperty()
128+
|
129+
this.asExpr() = p.getGetter().getACall()
130+
)
131+
}
132+
133+
override string getSourceType() { result = "ASP.NET Core component route parameter" }
134+
}
135+
}

csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll

+2-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ abstract class RemoteFlowSource extends SourceNode {
2626
* A module for importing frameworks that defines remote flow sources.
2727
*/
2828
private module RemoteFlowSources {
29-
private import semmle.code.csharp.frameworks.ServiceStack
29+
private import semmle.code.csharp.frameworks.ServiceStack as ServiceStack
30+
private import semmle.code.csharp.frameworks.microsoft.aspnetcore.Components as Blazor
3031
}
3132

3233
/** A data flow source of remote user input (ASP.NET). */

0 commit comments

Comments
 (0)