From 3e11b2e6e6b511b9fd945dcdfffacf5240e5d83b Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 30 Mar 2026 11:24:55 +0200 Subject: [PATCH 1/7] C#: Update remote flow sources test to also report tainted members. --- .../dataflow/flowsources/remote/remoteFlowSource.expected | 2 ++ .../dataflow/flowsources/remote/remoteFlowSource.ql | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected index f5f541d73d53..d115e08c8634 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected +++ b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected @@ -1,3 +1,5 @@ +remoteFlowSourceMembers +remoteFlowSources | Controller.cs:11:43:11:52 | sampleData | ASP.NET MVC action method parameter | | Controller.cs:11:62:11:66 | taint | ASP.NET MVC action method parameter | | Controller.cs:16:43:16:52 | sampleData | ASP.NET MVC action method parameter | diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql index fdea5323d5cb..f6d87eb9ff4f 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql +++ b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.ql @@ -1,5 +1,7 @@ import semmle.code.csharp.security.dataflow.flowsources.Remote -from RemoteFlowSource source -where source.getLocation().getFile().fromSource() -select source, source.getSourceType() +query predicate remoteFlowSourceMembers(TaintTracking::TaintedMember m) { m.fromSource() } + +query predicate remoteFlowSources(RemoteFlowSource source, string type) { + source.getLocation().getFile().fromSource() and type = source.getSourceType() +} From eced198a58378bb0d68cc96ad53c9c31cb8c8b4a Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 30 Mar 2026 12:53:17 +0200 Subject: [PATCH 2/7] C#: Reclassify some sources as AspNetRemoteFlowSource. --- .../code/csharp/security/dataflow/flowsources/Remote.qll | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll index 2906fde4e1de..e2ec595cd6d4 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll @@ -104,7 +104,7 @@ class WcfRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode { } /** A data flow source of remote user input (ASP.NET web service). */ -class AspNetServiceRemoteFlowSource extends RemoteFlowSource, DataFlow::ParameterNode { +class AspNetServiceRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ParameterNode { AspNetServiceRemoteFlowSource() { exists(Method m | m.getAParameter() = this.getParameter() and @@ -116,7 +116,8 @@ class AspNetServiceRemoteFlowSource extends RemoteFlowSource, DataFlow::Paramete } /** A data flow source of remote user input (ASP.NET request message). */ -class SystemNetHttpRequestMessageRemoteFlowSource extends RemoteFlowSource, DataFlow::ExprNode { +class SystemNetHttpRequestMessageRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode +{ SystemNetHttpRequestMessageRemoteFlowSource() { this.getType() instanceof SystemWebHttpRequestMessageClass } @@ -166,7 +167,7 @@ class MicrosoftOwinRequestRemoteFlowSource extends RemoteFlowSource, DataFlow::E } /** A parameter to an Mvc controller action method, viewed as a source of remote user input. */ -class ActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode { +class ActionMethodParameter extends AspNetRemoteFlowSource, DataFlow::ParameterNode { ActionMethodParameter() { exists(Parameter p | p = this.getParameter() and From f859fc47c44410bf0036a8eb12df3ccf68e8d5a3 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 30 Mar 2026 14:48:41 +0200 Subject: [PATCH 3/7] C#: Taint members of types used in ASP.NET remote flow source context. --- .../security/dataflow/flowsources/Remote.qll | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll index e2ec595cd6d4..2aa18c0d2a80 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll @@ -115,6 +115,40 @@ class AspNetServiceRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::Pa override string getSourceType() { result = "ASP.NET web service input" } } +/** + * Taint members (transitively) on types used in + * 1. Action method parameters. + * 2. WebMethod parameters. + * + * Note, that this also impacts uses of such types in other contexts. + */ +private class AspNetRemoteFlowSourceMember extends TaintTracking::TaintedMember { + AspNetRemoteFlowSourceMember() { + exists(Type t, Type t0 | t = this.getDeclaringType() | + (t = t0 or t = t0.(ArrayType).getElementType()) and + ( + t0 = any(AspNetRemoteFlowSourceMember m).getType() + or + t0 = any(ActionMethodParameter p).getType() + or + t0 = any(AspNetServiceRemoteFlowSource source).getType() + ) + ) and + this.isPublic() and + not this.isStatic() and + ( + this = + any(Property p | + p.isAutoImplemented() and + p.getGetter().isPublic() and + p.getSetter().isPublic() + ) + or + this = any(Field f | f.isPublic()) + ) + } +} + /** A data flow source of remote user input (ASP.NET request message). */ class SystemNetHttpRequestMessageRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::ExprNode { From cb7763addf70a1d52338cd5e8d9647cff2f7f62e Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Mon, 30 Mar 2026 12:44:07 +0200 Subject: [PATCH 4/7] C#: Add an ASP.NET flow source example when using the WebMethod attribute. --- .../flowsources/remote/RemoteFlowSource.cs | 48 +++++++++++++++++++ .../remote/remoteFlowSource.expected | 10 ++++ csharp/ql/test/resources/stubs/System.Web.cs | 5 ++ 3 files changed, 63 insertions(+) diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs b/csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs index 5889183f5257..3c7cbcba04a2 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs +++ b/csharp/ql/test/library-tests/dataflow/flowsources/remote/RemoteFlowSource.cs @@ -54,3 +54,51 @@ public static async void M3(System.Net.WebSockets.WebSocket webSocket) } } } + +namespace AspRemoteFlowSource +{ + using System.Web.Services; + + public class MySubData + { + public string SubDataProp { get; set; } + } + + public class MyElementSubData + { + public string ElementSubDataProp { get; set; } + } + + public class MyData + { + public string DataField; + public string DataProp { get; set; } + public MySubData SubData { get; set; } + public MyElementSubData[] Elements { get; set; } + } + + public class MyDataElement + { + public string Prop { get; set; } + } + + + public class MyService + { + [WebMethod] + public void MyMethod(MyData data) + { + Use(data.DataProp); + Use(data.SubData.SubDataProp); + Use(data.Elements[0].ElementSubDataProp); + } + + [WebMethod] + public void MyMethod2(MyDataElement[] data) + { + Use(data[0].Prop); + } + + public static void Use(object o) { } + } +} diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected index d115e08c8634..ef70ca9ad93a 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected +++ b/csharp/ql/test/library-tests/dataflow/flowsources/remote/remoteFlowSource.expected @@ -1,4 +1,12 @@ remoteFlowSourceMembers +| Controller.cs:6:19:6:25 | Tainted | +| RemoteFlowSource.cs:64:23:64:33 | SubDataProp | +| RemoteFlowSource.cs:69:23:69:40 | ElementSubDataProp | +| RemoteFlowSource.cs:74:23:74:31 | DataField | +| RemoteFlowSource.cs:75:23:75:30 | DataProp | +| RemoteFlowSource.cs:76:26:76:32 | SubData | +| RemoteFlowSource.cs:77:35:77:42 | Elements | +| RemoteFlowSource.cs:82:23:82:26 | Prop | remoteFlowSources | Controller.cs:11:43:11:52 | sampleData | ASP.NET MVC action method parameter | | Controller.cs:11:62:11:66 | taint | ASP.NET MVC action method parameter | @@ -12,3 +20,5 @@ remoteFlowSources | RemoteFlowSource.cs:45:17:45:23 | access to parameter request | ASP.NET query string | | RemoteFlowSource.cs:45:17:45:42 | access to property RawUrl | ASP.NET unvalidated request data | | RemoteFlowSource.cs:52:55:52:61 | [post] access to local variable segment | external | +| RemoteFlowSource.cs:89:37:89:40 | data | ASP.NET web service input | +| RemoteFlowSource.cs:97:47:97:50 | data | ASP.NET web service input | diff --git a/csharp/ql/test/resources/stubs/System.Web.cs b/csharp/ql/test/resources/stubs/System.Web.cs index c15b871095ff..23ae0f298cf4 100644 --- a/csharp/ql/test/resources/stubs/System.Web.cs +++ b/csharp/ql/test/resources/stubs/System.Web.cs @@ -454,3 +454,8 @@ public class SimpleTypeResolver : System.Web.Script.Serialization.JavaScriptType public SimpleTypeResolver() => throw null; } } + +namespace System.Web.Services +{ + public class WebMethodAttribute : Attribute { } +} From a26c5d1a843b1d9a88bf9a0ed60155a7a016639e Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 Apr 2026 11:06:15 +0200 Subject: [PATCH 5/7] C#: Streamline the implementation for ASP.NET Core tainted members. --- .../security/dataflow/flowsources/Remote.qll | 51 +++++++++++-------- .../aspremote/AspRemoteFlowSource.cs | 2 +- .../aspremote/aspRemoteFlowSource.expected | 1 + 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll index 2aa18c0d2a80..4fab2e8f5482 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll @@ -115,6 +115,23 @@ class AspNetServiceRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::Pa override string getSourceType() { result = "ASP.NET web service input" } } +private class CandidateMembersToTaint extends Member { + CandidateMembersToTaint() { + this.isPublic() and + not this.isStatic() and + ( + this = + any(Property p | + p.isAutoImplemented() and + p.getGetter().isPublic() and + p.getSetter().isPublic() + ) + or + this = any(Field f | f.isPublic()) + ) + } +} + /** * Taint members (transitively) on types used in * 1. Action method parameters. @@ -122,7 +139,9 @@ class AspNetServiceRemoteFlowSource extends AspNetRemoteFlowSource, DataFlow::Pa * * Note, that this also impacts uses of such types in other contexts. */ -private class AspNetRemoteFlowSourceMember extends TaintTracking::TaintedMember { +private class AspNetRemoteFlowSourceMember extends TaintTracking::TaintedMember, + CandidateMembersToTaint +{ AspNetRemoteFlowSourceMember() { exists(Type t, Type t0 | t = this.getDeclaringType() | (t = t0 or t = t0.(ArrayType).getElementType()) and @@ -133,18 +152,6 @@ private class AspNetRemoteFlowSourceMember extends TaintTracking::TaintedMember or t0 = any(AspNetServiceRemoteFlowSource source).getType() ) - ) and - this.isPublic() and - not this.isStatic() and - ( - this = - any(Property p | - p.isAutoImplemented() and - p.getGetter().isPublic() and - p.getSetter().isPublic() - ) - or - this = any(Field f | f.isPublic()) ) } } @@ -253,14 +260,18 @@ class AspNetCoreRoutingMethodParameter extends AspNetCoreRemoteFlowSource, DataF * Flow is defined from any ASP.NET Core remote source object to any of its member * properties. */ -private class AspNetCoreRemoteFlowSourceMember extends TaintTracking::TaintedMember, Property { +private class AspNetCoreRemoteFlowSourceMember extends TaintTracking::TaintedMember, + CandidateMembersToTaint +{ AspNetCoreRemoteFlowSourceMember() { - this.getDeclaringType() = any(AspNetCoreRemoteFlowSource source).getType() and - this.isPublic() and - not this.isStatic() and - this.isAutoImplemented() and - this.getGetter().isPublic() and - this.getSetter().isPublic() + exists(Type t, Type t0 | t = this.getDeclaringType() | + (t = t0 or t = t0.(ArrayType).getElementType()) and + ( + t0 = any(AspNetCoreRemoteFlowSourceMember m).getType() + or + t0 = any(AspNetCoreRemoteFlowSource m).getType() + ) + ) } } diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs b/csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs index 176f95e4a89d..e554f25f2064 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs +++ b/csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs @@ -8,7 +8,7 @@ namespace Testing public class ViewModel { public string RequestId { get; set; } // Considered tainted. - public object RequestIdField; // Not considered tainted as it is a field. + public object RequestIdField; // Considered tainted. public string RequestIdOnlyGet { get; } // Not considered tainted as there is no setter. public string RequestIdPrivateSet { get; private set; } // Not considered tainted as it has a private setter. public static object RequestIdStatic { get; set; } // Not considered tainted as it is static. diff --git a/csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected b/csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected index a7442a80839c..d729eb939d28 100644 --- a/csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected +++ b/csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected @@ -1,5 +1,6 @@ remoteFlowSourceMembers | AspRemoteFlowSource.cs:10:23:10:31 | RequestId | +| AspRemoteFlowSource.cs:11:23:11:36 | RequestIdField | | AspRemoteFlowSource.cs:28:23:28:29 | Tainted | remoteFlowSources | AspRemoteFlowSource.cs:20:42:20:50 | viewModel | From 2d4c18ed58fe70f785bccccdd6e946626bbc9370 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 Apr 2026 11:23:12 +0200 Subject: [PATCH 6/7] C#: Add change-note. --- csharp/ql/lib/change-notes/2026-04-01-asp-remote-sources.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 csharp/ql/lib/change-notes/2026-04-01-asp-remote-sources.md diff --git a/csharp/ql/lib/change-notes/2026-04-01-asp-remote-sources.md b/csharp/ql/lib/change-notes/2026-04-01-asp-remote-sources.md new file mode 100644 index 000000000000..52f3f721e9fa --- /dev/null +++ b/csharp/ql/lib/change-notes/2026-04-01-asp-remote-sources.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Expanded ASP and ASP.NET remote source modeling to cover additional sources, including fields of tainted parameters as well as properties and fields that become tainted transitively. From 0a23104000ecf1bb04cb6bef0111b21c0b021ba7 Mon Sep 17 00:00:00 2001 From: Michael Nebel Date: Wed, 1 Apr 2026 13:23:06 +0200 Subject: [PATCH 7/7] C#: Address review comments. --- .../semmle/code/csharp/security/dataflow/flowsources/Remote.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll index 4fab2e8f5482..2a74c7844b12 100644 --- a/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll +++ b/csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsources/Remote.qll @@ -137,7 +137,7 @@ private class CandidateMembersToTaint extends Member { * 1. Action method parameters. * 2. WebMethod parameters. * - * Note, that this also impacts uses of such types in other contexts. + * Note that this also impacts uses of such types in other contexts. */ private class AspNetRemoteFlowSourceMember extends TaintTracking::TaintedMember, CandidateMembersToTaint