C#: Add Razor Page handler method parameters as remote flow sources#21984
Open
felickz wants to merge 2 commits into
Open
C#: Add Razor Page handler method parameters as remote flow sources#21984felickz wants to merge 2 commits into
felickz wants to merge 2 commits into
Conversation
ASP.NET Core Razor Page handler method parameters (OnGet, OnPost, etc.) were not modeled as remote flow sources, causing security queries like SQL injection to miss vulnerabilities in PageModel subclasses. This adds AspNetCorePageHandlerMethodParameter, analogous to the existing AspNetCoreActionMethodParameter for MVC controllers, using the existing PageModelClass.getAHandlerMethod() from Razor.qll. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds CodeQL remote flow sources for ASP.NET Core Razor Pages handler method parameters, with accompanying library tests to validate detection.
Changes:
- Introduces a new
AspNetCorePageHandlerMethodParameterremote flow source inRemote.qll. - Adds Razor Pages
PageModelhandler-method test cases to the C# test fixture. - Updates the expected test output to include Razor Page handler parameters as remote sources.
Show a summary per file
| File | Description |
|---|---|
| csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll | Adds Razor framework import and a new remote flow source for Razor Page handler method parameters. |
| csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs | Adds Razor PageModel handler methods (and a derived PageModel) to exercise the new flow source. |
| csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected | Updates expected results to include the newly-detected Razor handler parameters. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
ASP.NET Core Razor Page handler method parameters (
OnGet,OnPost, etc.) were not modeled as remote flow sources, causing security queries like SQL injection to miss vulnerabilities inPageModelsubclasses.What we now detect as sources
Problem
AspNetCoreActionMethodParameterinRemote.qllonly coversMicrosoftAspNetCoreMvcController.getAnActionMethod().getAParameter()— MVC controllers only. Razor Pages extendPageModel, not a controller, so handler method parameters likeOnPost([FromForm] string command)were invisible to taint tracking.This was discovered when a known SQL injection vulnerability in a Razor Page was not detected by
cs/sql-injection.Fix
Adds
AspNetCorePageHandlerMethodParameter, analogous toAspNetCoreActionMethodParameter, using the existingPageModelClass.getAHandlerMethod()fromRazor.qll. This properly handles:OnGet,OnPost,OnPut,OnDelete, etc.) and async variantsPageModel(including derived classes)[NonHandler]exclusions (via the existinggetAHandlerMethod()predicate)Testing
OnGet,OnPost,OnPostAsync,OnPut,OnDeletehandler parameters[NonHandler]-attributed methodsDerivedPageModelextendingMyPageModel)aspremoteandremoteflow source tests pass with no regressions