mirror of
https://github.com/github/codeql.git
synced 2026-06-19 03:41:07 +02:00
C#: Add Razor Page handler method parameters as remote flow sources
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>
This commit is contained in:
@@ -13,6 +13,7 @@ private import semmle.code.csharp.frameworks.system.web.ui.WebControls
|
||||
private import semmle.code.csharp.frameworks.WCF
|
||||
private import semmle.code.csharp.frameworks.microsoft.Owin
|
||||
private import semmle.code.csharp.frameworks.microsoft.AspNetCore
|
||||
private import semmle.code.csharp.frameworks.Razor
|
||||
private import semmle.code.csharp.dataflow.internal.ExternalFlow
|
||||
private import semmle.code.csharp.security.dataflow.flowsources.FlowSources
|
||||
|
||||
@@ -314,6 +315,22 @@ class AspNetCoreActionMethodParameter extends AspNetCoreRemoteFlowSource, DataFl
|
||||
override string getSourceType() { result = "ASP.NET Core MVC action method parameter" }
|
||||
}
|
||||
|
||||
/** A parameter to a Razor Page handler method, viewed as a source of remote user input. */
|
||||
class AspNetCorePageHandlerMethodParameter extends AspNetCoreRemoteFlowSource,
|
||||
DataFlow::ParameterNode
|
||||
{
|
||||
AspNetCorePageHandlerMethodParameter() {
|
||||
exists(Parameter p |
|
||||
p = this.getParameter() and
|
||||
p.fromSource()
|
||||
|
|
||||
p = any(PageModelClass pm).getAHandlerMethod().getAParameter()
|
||||
)
|
||||
}
|
||||
|
||||
override string getSourceType() { result = "ASP.NET Core Razor Page handler method parameter" }
|
||||
}
|
||||
|
||||
private class ExternalRemoteFlowSource extends RemoteFlowSource {
|
||||
ExternalRemoteFlowSource() { sourceNode(this, "remote") }
|
||||
|
||||
|
||||
@@ -63,4 +63,32 @@ namespace Testing
|
||||
{
|
||||
public void MyActionMethod(string param) { }
|
||||
}
|
||||
|
||||
// Razor Page handler tests
|
||||
public class MyPageModel : Microsoft.AspNetCore.Mvc.RazorPages.PageModel
|
||||
{
|
||||
// BAD: handler method parameters are user-controlled
|
||||
public void OnGet(string id) { }
|
||||
|
||||
public void OnPost(string command, int count) { }
|
||||
|
||||
public void OnPostAsync(string data) { }
|
||||
|
||||
public void OnPut(string value) { }
|
||||
|
||||
public void OnDelete(string itemId) { }
|
||||
|
||||
// GOOD: not a handler method (doesn't start with On)
|
||||
public void GetUser(string userId) { }
|
||||
|
||||
// GOOD: marked with NonHandler attribute
|
||||
[Microsoft.AspNetCore.Mvc.RazorPages.NonHandlerAttribute]
|
||||
public void OnGetNonHandler(string param) { }
|
||||
}
|
||||
|
||||
// Subclass of a PageModel subclass
|
||||
public class DerivedPageModel : MyPageModel
|
||||
{
|
||||
public void OnPost(string derivedParam) { }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14,3 +14,10 @@ remoteFlowSources
|
||||
| AspRemoteFlowSource.cs:54:69:54:82 | mapDeleteParam |
|
||||
| AspRemoteFlowSource.cs:56:41:56:44 | item |
|
||||
| AspRemoteFlowSource.cs:64:43:64:47 | param |
|
||||
| AspRemoteFlowSource.cs:71:34:71:35 | id |
|
||||
| AspRemoteFlowSource.cs:73:35:73:41 | command |
|
||||
| AspRemoteFlowSource.cs:73:48:73:52 | count |
|
||||
| AspRemoteFlowSource.cs:75:40:75:43 | data |
|
||||
| AspRemoteFlowSource.cs:77:34:77:38 | value |
|
||||
| AspRemoteFlowSource.cs:79:37:79:42 | itemId |
|
||||
| AspRemoteFlowSource.cs:92:35:92:46 | derivedParam |
|
||||
|
||||
Reference in New Issue
Block a user