-
Notifications
You must be signed in to change notification settings - Fork 1.9k
C#: Blazor: Add route parameters as remote flow sources #18664
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
Merged
egregius313
merged 12 commits into
github:main
from
egregius313:egregius313/csharp/blazor/url-param-sources
Feb 7, 2025
Merged
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
8bae79d
Components file
egregius313 6ae7ede
Add Blazor components file to Remote sources module
egregius313 aaefa0f
Add remote flow source test
egregius313 414c0a6
Fix test results
egregius313 5236a40
Remove unnecessary class
egregius313 eb25c76
Change note
egregius313 07aad61
Typo
egregius313 12ebfa6
Change join order of Property/Name matching
egregius313 a783ac1
Add QL tests for remoteFlowSource
egregius313 274a2d8
Remove remoteFlowSource integration test
egregius313 0a817eb
Fix test expectations
egregius313 29d03db
Remove unneeded disjunction
egregius313 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
4 changes: 4 additions & 0 deletions
4
csharp/ql/integration-tests/all-platforms/blazor/remoteFlowSource.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| | BlazorTest/Components/Pages/TestPage.razor:10:29:10:36 | access to property UrlParam | ASP.NET Core component route parameter | | ||
| | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | ASP.NET Core component route parameter | | ||
| | BlazorTest/Components/Pages/TestPage.razor:19:38:19:47 | access to property QueryParam | external | | ||
| | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | external | |
7 changes: 7 additions & 0 deletions
7
csharp/ql/integration-tests/all-platforms/blazor/remoteFlowSource.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import semmle.code.csharp.security.dataflow.flowsources.Remote | ||
|
|
||
| from RemoteFlowSource source, File f | ||
| where | ||
| source.getLocation().getFile() = f and | ||
| (f.fromSource() or f.getExtension() = "razor") | ||
| select source, source.getSourceType() | ||
4 changes: 4 additions & 0 deletions
4
csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/remoteFlowSource.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| | BlazorTest/Components/Pages/TestPage.razor:10:29:10:36 | access to property UrlParam | ASP.NET Core component route parameter | | ||
| | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | ASP.NET Core component route parameter | | ||
| | BlazorTest/Components/Pages/TestPage.razor:19:38:19:47 | access to property QueryParam | external | | ||
| | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | external | |
7 changes: 7 additions & 0 deletions
7
csharp/ql/integration-tests/all-platforms/blazor_build_mode_none/remoteFlowSource.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import semmle.code.csharp.security.dataflow.flowsources.Remote | ||
|
|
||
| from RemoteFlowSource source, File f | ||
| where | ||
| source.getLocation().getFile() = f and | ||
| (f.fromSource() or f.getExtension() = "razor") | ||
| select source, source.getSourceType() |
4 changes: 4 additions & 0 deletions
4
csharp/ql/integration-tests/all-platforms/blazor_net_8/remoteFlowSource.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| | BlazorTest/Components/Pages/TestPage.razor:10:29:10:36 | access to property UrlParam | ASP.NET Core component route parameter | | ||
| | BlazorTest/Components/Pages/TestPage.razor:11:48:11:55 | access to property UrlParam | ASP.NET Core component route parameter | | ||
| | BlazorTest/Components/Pages/TestPage.razor:19:38:19:47 | access to property QueryParam | external | | ||
| | BlazorTest/Components/Pages/TestPage.razor:20:60:20:69 | access to property QueryParam | external | |
7 changes: 7 additions & 0 deletions
7
csharp/ql/integration-tests/all-platforms/blazor_net_8/remoteFlowSource.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| import semmle.code.csharp.security.dataflow.flowsources.Remote | ||
|
|
||
| from RemoteFlowSource source, File f | ||
| where | ||
| source.getLocation().getFile() = f and | ||
| (f.fromSource() or f.getExtension() = "razor") | ||
| select source, source.getSourceType() |
136 changes: 136 additions & 0 deletions
136
csharp/ql/lib/semmle/code/csharp/frameworks/microsoft/aspnetcore/Components.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| /** Provides classes for working with `Microsoft.AspNetCore.Components` */ | ||
|
|
||
| import csharp | ||
| import semmle.code.csharp.frameworks.Microsoft | ||
Check warningCode scanning / CodeQL Redundant import Warning
Redundant import, the module is already imported inside
semmle.code.csharp.frameworks.microsoft.AspNetCore Error loading related location Loading |
||
| import semmle.code.csharp.frameworks.microsoft.AspNetCore | ||
|
|
||
| /** The `Microsoft.AspNetCore.Components` namespace */ | ||
| class MicrosoftAspNetCoreComponentsNamespace extends Namespace { | ||
| MicrosoftAspNetCoreComponentsNamespace() { | ||
| this.getParentNamespace() instanceof MicrosoftAspNetCoreNamespace and | ||
| this.hasName("Components") | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A class in the `Microsoft.AspNetCore.Components` namespace. | ||
| */ | ||
| private class MicrosoftAspNetCoreComponentsClass extends Class { | ||
| MicrosoftAspNetCoreComponentsClass() { | ||
| this.getNamespace() instanceof MicrosoftAspNetCoreComponentsNamespace | ||
| } | ||
| } | ||
|
|
||
| /** The `Microsoft.AspNetCore.Components.ParameterAttribute` class. */ | ||
| class MicrosoftAspNetCoreComponentsParamaterAttributeClass extends MicrosoftAspNetCoreComponentsClass | ||
|
||
| { | ||
| MicrosoftAspNetCoreComponentsParamaterAttributeClass() { this.hasName("ParameterAttribute") } | ||
| } | ||
|
|
||
| /** The `Microsoft.AspNetCore.Components.CascadingParameterAttributeBase` class. */ | ||
| class MicrosoftAspNetCoreComponentsCascadingParameterAttributeBaseClass extends MicrosoftAspNetCoreComponentsClass | ||
| { | ||
| MicrosoftAspNetCoreComponentsCascadingParameterAttributeBaseClass() { | ||
| this.hasName("CascadingParameterAttributeBase") | ||
| } | ||
| } | ||
|
|
||
| /** The `Microsoft.AspNetCore.Components.ComponentBase` class. */ | ||
| class MicrosoftAspNetCoreComponentsComponentBaseClass extends MicrosoftAspNetCoreComponentsClass { | ||
| MicrosoftAspNetCoreComponentsComponentBaseClass() { this.hasName("ComponentBase") } | ||
| } | ||
|
|
||
| /** The `Microsoft.AspNetCore.Components.IComponent` interface. */ | ||
| class MicrosoftAspNetCoreComponentsIComponentInterface extends Interface { | ||
| MicrosoftAspNetCoreComponentsIComponentInterface() { | ||
| this.getNamespace() instanceof MicrosoftAspNetCoreComponentsNamespace and | ||
| this.hasName("IComponent") | ||
| } | ||
| } | ||
|
|
||
| /** The `Microsoft.AspNetCore.Components.RouteAttribute` attribute. */ | ||
| private class MicrosoftAspNetCoreComponentsRouteAttribute extends Attribute { | ||
| MicrosoftAspNetCoreComponentsRouteAttribute() { | ||
| this.getType().getNamespace() instanceof MicrosoftAspNetCoreComponentsNamespace and | ||
| this.getType().hasName("RouteAttribute") | ||
| } | ||
| } | ||
|
|
||
| /** The `Microsoft.AspNetCore.Components.ParameterAttribute` attribute. */ | ||
| private class MicrosoftAspNetCoreComponentsParameterAttribute extends Attribute { | ||
| MicrosoftAspNetCoreComponentsParameterAttribute() { | ||
| this.getType().getNamespace() instanceof MicrosoftAspNetCoreComponentsNamespace and | ||
| this.getType().hasName("ParameterAttribute") | ||
| } | ||
| } | ||
|
|
||
| /** An ASP.NET Core (Blazor) component. */ | ||
| class MicrosoftAspNetCoreComponentsComponent extends Class { | ||
| MicrosoftAspNetCoreComponentsComponent() { | ||
| this.getABaseType+() instanceof MicrosoftAspNetCoreComponentsComponentBaseClass or | ||
| this.getABaseType+() instanceof MicrosoftAspNetCoreComponentsIComponentInterface | ||
| } | ||
|
|
||
| /** Gets a property whose value cascades down the component hierarchy. */ | ||
| Property getACascadingParameterProperty() { | ||
| result = this.getAProperty() and | ||
| result.getAnAttribute().getType().getBaseClass() instanceof | ||
| MicrosoftAspNetCoreComponentsCascadingParameterAttributeBaseClass | ||
| } | ||
|
|
||
| /** Gets the url for the route from the `Microsoft.AspNetCore.Components.RouteAttribute` of the component. */ | ||
| private string getRouteAttributeUrl() { | ||
| exists(MicrosoftAspNetCoreComponentsRouteAttribute a | a = this.getAnAttribute() | | ||
| result = a.getArgument(0).getValue() | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Gets a route parameter from the `Microsoft.AspNetCore.Components.RouteAttribute` of the component. | ||
| * | ||
| * A route parameter is defined in the URL by wrapping its name in a pair of { braces } when adding a component's @page declaration. | ||
| * There are various extensions that can be added next to the parameter name, such as `:int` or `?` to make the parameter optional. | ||
| * Optionally, the parameter name can start with a `*` to make it a catch-all parameter. | ||
| * | ||
| * And example of a route parameter is `@page "/counter/{id:int}/{other?}/{*rest}"`, from this we're getting the `id`, `other` and `rest` parameters. | ||
egregius313 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| */ | ||
| private string getARouteParameter() { | ||
| result = this.getRouteAttributeUrl().splitAt("{").regexpCapture("\\*?([^:?}]+)[:?}](.*)", 1) | ||
| } | ||
|
|
||
| /** Gets a property attributed with `[Parameter]` attribute. */ | ||
| Property getAParameterProperty() { | ||
| result = this.getAProperty() and | ||
| result.getAnAttribute() instanceof MicrosoftAspNetCoreComponentsParameterAttribute | ||
| } | ||
|
|
||
| /** Gets a property whose value is populated from route parameters. */ | ||
| Property getARouteParameterProperty() { | ||
| result = this.getAParameterProperty() and | ||
| exists(string urlParamName | urlParamName = this.getARouteParameter() | | ||
| result.getName().toLowerCase() = urlParamName.toLowerCase() | ||
| ) | ||
| } | ||
| } | ||
|
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. Joining on names is always a candidate for a bad join, so it is usually safer to force multi-column joins, e.g. pragma[nomagic]
private Property getAParameterProperty(string name) {
result = this.getAProperty() and
result.getAnAttribute() instanceof MicrosoftAspNetCoreComponentsParameterAttribute and
name = result.getName().toLowerCase()
}
pragma[nomagic]
private string getARouteParameter() {
exists(string s |
s = this.getRouteAttributeUrl().splitAt("{").regexpCapture("\\*?([^:?}]+)[:?}](.*)", 1) and
result = s.toLowerCase()
)
}
Property getARouteParameterProperty() {
exists(string name |
result = this.getAParameterProperty(name) and
name = this.getARouteParameter()
)
} |
||
|
|
||
| private module Sources { | ||
| private import semmle.code.csharp.security.dataflow.flowsources.Remote | ||
|
|
||
| /** | ||
| * A property with a `[Parameter]` attribute in an ASP.NET Core component which | ||
| * is populated from a route parameter. | ||
| */ | ||
| private class AspNetCoreComponentRouteParameterFlowSource extends AspNetRemoteFlowSource, | ||
| DataFlow::ExprNode | ||
| { | ||
| AspNetCoreComponentRouteParameterFlowSource() { | ||
| exists(MicrosoftAspNetCoreComponentsComponent c, Property p | | ||
| p = c.getARouteParameterProperty() | ||
| | | ||
| this.asExpr() = p.getGetter().getACall() | ||
| ) | ||
| } | ||
|
|
||
| override string getSourceType() { result = "ASP.NET Core component route parameter" } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
What is the motivation for using integration tests instead of normal QL tests? I find it quite difficult to run
(specific) integration tests locally...
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.
This was mostly because the test was originally from Tamás's WIP PR. And I wasn't sure about testing Blazor components as a QL test (I'm unsure if I'll need to have a full project). I'll experiment with moving it.
You are correct about the integration test being a more complicated way to run the test locally.
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.
If it's not easy to do, then let's do only the integration test.