Merge pull request #9393 from michaelnebel/csharp/asptaintedmember

C#: ASP.NET Core like members are tainted

Author: michaelnebel

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

@@ -171,6 +171,23 @@ class ActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode {
 /** A data flow source of remote user input (ASP.NET Core). */
 abstract class AspNetCoreRemoteFlowSource extends RemoteFlowSource { }
 
+/**
+ * Data flow for AST.NET Core.
+ *
+ * Flow is defined from any ASP.NET Core remote source object to any of its member
+ * properties.
+ */
+private class AspNetCoreRemoteFlowSourceMember extends TaintTracking::TaintedMember {
+  AspNetCoreRemoteFlowSourceMember() {
+    this.getDeclaringType() = any(AspNetCoreRemoteFlowSource source).getType() and
+    this.isPublic() and
+    not this.isStatic() and
+    exists(Property p | p = this |
+      p.isAutoImplemented() and p.getGetter().isPublic() and p.getSetter().isPublic()
+    )
+  }
+}
+
 /** A data flow source of remote user input (ASP.NET query collection). */
 class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFlow::ExprNode {
   AspNetCoreQueryRemoteFlowSource() {
@@ -196,7 +213,7 @@ class AspNetCoreQueryRemoteFlowSource extends AspNetCoreRemoteFlowSource, DataFl
 }
 
 /** A parameter to a `Mvc` controller action method, viewed as a source of remote user input. */
-class AspNetCoreActionMethodParameter extends RemoteFlowSource, DataFlow::ParameterNode {
+class AspNetCoreActionMethodParameter extends AspNetCoreRemoteFlowSource, DataFlow::ParameterNode {
   AspNetCoreActionMethodParameter() {
     exists(Parameter p |
       p = this.getParameter() and

csharp/ql/src/change-notes/2022-06-02-aspnetcoretaintedmembers.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* All auto implemented public properties with public getters and setters on ASP.NET Core remote flow sources are now also considered to be tainted.

csharp/ql/test/library-tests/dataflow/flowsources/aspremote/AspRemoteFlowSource.cs

@@ -0,0 +1,23 @@
+using Microsoft.AspNetCore.Mvc;
+
+namespace Testing
+{
+
+    public class ViewModel
+    {
+        public string RequestId { get; set; } // Considered tainted.
+        public object RequestIdField; // Not considered tainted as it is a field.
+        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.
+        private string RequestIdPrivate { get; set; } // Not considered tainted as it is private.
+    }
+
+    public class TestController : Controller
+    {
+        public object MyAction(ViewModel viewModel)
+        {
+            throw null;
+        }
+    }
+}

csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.expected

@@ -0,0 +1,4 @@
+remoteFlowSourceMembers
+| AspRemoteFlowSource.cs:8:23:8:31 | RequestId |
+remoteFlowSources
+| AspRemoteFlowSource.cs:18:42:18:50 | viewModel |

csharp/ql/test/library-tests/dataflow/flowsources/aspremote/aspRemoteFlowSource.ql

@@ -0,0 +1,8 @@
+import csharp
+import semmle.code.csharp.security.dataflow.flowsources.Remote
+
+query predicate remoteFlowSourceMembers(TaintTracking::TaintedMember m) { m.fromSource() }
+
+query predicate remoteFlowSources(AspNetCoreRemoteFlowSource s) {
+  s.getEnclosingCallable().fromSource()
+}

csharp/ql/test/library-tests/dataflow/flowsources/aspremote/options

@@ -0,0 +1,4 @@
+semmle-extractor-options: /nostdlib /noconfig
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/Microsoft.Extensions.Primitives/6.0.0/Microsoft.Extensions.Primitives.csproj
+semmle-extractor-options: ${testdir}/../../../../resources/stubs/AspNetCore.cs

csharp/ql/test/query-tests/Security Features/CWE-079/XSS/options

@@ -1,3 +1,4 @@
 semmle-extractor-options: /nostdlib /noconfig
 semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/_frameworks/Microsoft.NETCore.App/Microsoft.NETCore.App.csproj
-semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/Microsoft.Extensions.Primitives/6.0.0/Microsoft.Extensions.Primitives.csproj
+semmle-extractor-options: --load-sources-from-project:${testdir}/../../../../resources/stubs/Microsoft.Extensions.Primitives/6.0.0/Microsoft.Extensions.Primitives.csproj
+semmle-extractor-options: ${testdir}/../../../../resources/stubs/AspNetCore.cs