Create rule S6932: Use model binding instead of reading raw request data

rules/S6932/S6932.AspNet4x.cs

@@ -0,0 +1,109 @@
+using System.Web;
+using System.Web.Http;
+using System.Web.Mvc;
+
+namespace WebApplication2.Controllers
+{
+    // Hint: ApiController does not expose QueryString or Form directly. The rule does not apply there
+    public class TestController : Controller
+    {
+        public ActionResult Post()
+        {
+            _ = Request.Form["id"];                           // Noncompliant {{Use model binding instead of accessing the raw request data}}
+            //          ^^^^
+            _ = Request.Form.Get("id");                       // Noncompliant {{Use model binding instead of accessing the raw request data}}
+            //          ^^^^
+            _ = Request.Form.GetValues("id");                 // Noncompliant {{Use model binding instead of accessing the raw request data}}
+            //          ^^^^
+            _ = Request.QueryString["id"];                    // Noncompliant {{Use model binding instead of accessing the raw request data}}
+            //          ^^^^^^^^^^^
+            _ = Request.QueryString.Get("id");                // Noncompliant {{Use model binding instead of accessing the raw request data}}
+            //          ^^^^^^^^^^^
+            _ = Request.QueryString.GetValues("id");          // Noncompliant {{Use model binding instead of accessing the raw request data}}
+            //          ^^^^^^^^^^^
+            return default;
+        }
+
+        // Parameterized for "Form", "QueryString"
+        void NoncompliantKeyVariations()
+        {
+            _ = Request.Form[@"key"];                    // Noncompliant
+            _ = Request.Form.Get(@"key");                // Noncompliant
+            _ = Request.Form.GetValues(@"key");          // Noncompliant
+
+            const string key = "id";
+            _ = Request.Form[key];                       // Noncompliant
+            _ = Request.Form.Get(key);                   // Noncompliant
+            _ = Request.Form.GetValues(key);             // Noncompliant
+            _ = Request.Form[$"prefix.{key}"];           // Noncompliant
+            _ = Request.Form.Get($"prefix.{key}");       // Noncompliant
+            _ = Request.Form.GetValues($"prefix.{key}"); // Noncompliant
+
+            _ = Request.Form[name: "id"];                // Noncompliant
+            _ = Request.Form.Get(name: "id");            // Noncompliant
+            _ = Request.Form.GetValues(name: "id");      // Noncompliant
+        }
+
+        // Parameterized: "Form" and "QueryString" and 
+        void Compliant(string key)
+        {
+            // Compliant: Accessing by index is not supported by model binding
+            _ = Request.Form[0];
+            _ = Request.Form.Get(0);
+            _ = Request.Form.GetValues(0);
+            // Compliant: key is not a compile time constant
+            _ = Request.Form[key];
+            _ = Request.Form.Get(key);
+            _ = Request.Form.GetValues(key);
+
+            _ = Request.Form.AllKeys;
+            _ = Request.Form.Count;
+            _ = Request.Form.Keys;
+            _ = Request.Form.HasKeys();
+            _ = Request.Form.GetKey(0);
+            Request.Form.Set("id", "value"); ;
+            Request.Form.Add("id", "value");
+            Request.Form.Remove("id");
+        }
+
+        // parameterized test: parameters are the different forbidden Request accesses (see above)
+        private static void HandleRequest(HttpRequest request)
+        {
+            _ = request.Form["id"]; // Noncompliant: Containing type is a controller
+            void LocalFunction()
+            {
+                _ = request.Form["id"]; // Noncompliant: Containing type is a controller
+            }
+        }
+    }
+
+    // parameterized test: Repeat for Controller, MyBaseController, MyBaseBaseController base classes
+    public class MyBaseController : Controller { }
+    public class MyBaseBaseController : MyBaseController { }
+    public class MyTestController : MyBaseBaseController
+    {
+        public void Action()
+        {
+            _ = Request.Form["id"]; // Noncompliant
+        }
+    }
+
+    static class HttpRequestExtensions
+    {
+        // parameterized test: parameters are the different forbidden Request accesses (see above)
+        public static void Ext(this HttpRequest request)
+        {
+            _ = request.Form["id"]; // Compliant: Not in a controller
+        }
+    }
+
+    class RequestService
+    {
+        public HttpRequest Request { get; }
+        // parameterized test: parameters are the different forbidden Request accesses (see above)
+        public void HandleRequest(HttpRequest request)
+        {
+            _ = Request.Form["id"]; // Compliant: Not in a controller
+        }
+    }
+}

rules/S6932/S6932.AspNetCore.cs

@@ -0,0 +1,119 @@
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc;
+using System.Linq;
+using System.Threading.Tasks;
+
+namespace WebApplication1.Controllers;
+
+public class TestController : Controller
+{
+    public IActionResult Post()
+    {
+        _ = Request.Form["id"];                           // Noncompliant {{Use model binding instead of accessing the raw request data}}
+        //          ^^^^
+        _ = Request.Form.TryGetValue("id", out _);        // Noncompliant {{Use model binding instead of accessing the raw request data}}
+        //          ^^^^
+        _ = Request.Form.ContainsKey("id");               // Noncompliant {{Use model binding instead of accessing the raw request data}}
+        //          ^^^^
+        _ = Request.Query["id"];                          // Noncompliant {{Use model binding instead of accessing the raw request data}}
+        //          ^^^^^
+        _ = Request.Query.TryGetValue("id", out _);       // Noncompliant {{Use model binding instead of accessing the raw request data}}
+        //          ^^^^^
+        _ = Request.RouteValues["id"];                    // Noncompliant {{Use model binding instead of accessing the raw request data}}
+        //          ^^^^^^^^^^^
+        _ = Request.RouteValues.TryGetValue("id", out _); // Noncompliant {{Use model binding instead of accessing the raw request data}}
+        //          ^^^^^^^^^^^
+        _ = Request.Form.Files;                           // Noncompliant {{Use IFormFile or IFormFileCollection binding instead}}
+        //               ^^^^^
+        _ = Request.Form.Files["file"];                   // Noncompliant {{Use IFormFile or IFormFileCollection binding instead}}
+        //               ^^^^^
+        _ = Request.Form.Files[0];                        // Noncompliant {{Use IFormFile or IFormFileCollection binding instead}}
+        //               ^^^^^
+        _ = Request.Form.Files.Any();                     // Noncompliant {{Use IFormFile or IFormFileCollection binding instead}}
+        //               ^^^^^
+        _ = Request.Form.Files.Count;                     // Noncompliant {{Use IFormFile or IFormFileCollection binding instead}}
+        //               ^^^^^
+        _ = Request.Form.Files.GetFile("file");           // Noncompliant {{Use IFormFile or IFormFileCollection binding instead}}
+        //               ^^^^^
+        _ = Request.Form.Files.GetFiles("file");          // Noncompliant {{Use IFormFile or IFormFileCollection binding instead}}
+        //               ^^^^^
+        return default;
+    }
+
+    // Parameterized for "Form", "Query" and "RouteValues"
+    void NoncompliantKeyVariations()
+    {
+        _ = Request.Form[@"key"];                                 // Noncompliant
+        _ = Request.Form.TryGetValue(@"key", out _);              // Noncompliant
+        _ = Request.Form["""key"""];                              // Noncompliant
+        _ = Request.Form.TryGetValue("""key""", out _);           // Noncompliant
+
+        const string key = "id";
+        _ = Request.Form[key];                                    // Noncompliant
+        _ = Request.Form.TryGetValue(key, out _);                 // Noncompliant
+        _ = Request.Form[$"prefix.{key}"];                        // Noncompliant
+        _ = Request.Form.TryGetValue($"prefix.{key}", out _);     // Noncompliant
+        _ = Request.Form[$"""prefix.{key}"""];                    // Noncompliant
+        _ = Request.Form.TryGetValue($"""prefix.{key}""", out _); // Noncompliant
+
+        _ = Request.Form[key: "id"];                              // Noncompliant
+        _ = Request.Form.TryGetValue(value: out _, key: "id");    // Noncompliant
+    }
+
+    // Parameterized for "Form", "Query", and "RouteValues"
+    async Task Compliant(string key)
+    {
+        _ = Request.Form.Keys;
+        _ = Request.Form.Count;
+        foreach (var kvp in Request.Form) { }
+        _ = Request.Form.Select(x => x);
+        _ = Request.Form[key];                // Compliant: The accessed key is not a compile time constant
+        _ = Request.Cookies["coockie"];       // Compliant: Cookies are not bound by default
+        _ = Request.QueryString;              // FN: QueryString is the whole raw string. We should raise for Request.QueryString.Split() calls
+        _ = await Request.ReadFormAsync();    // Compliant: This might be used for optimization purposes e.g. conditional form value access.
+    }
+
+    // parameterized test: parameters are the different forbidden Request accesses (see above)
+    private static void HandleRequest(HttpRequest request)
+    {
+        _ = request.Form["id"]; // Noncompliant: Containing type is a controller
+        void LocalFunction()
+        {
+            _ = request.Form["id"]; // Noncompliant: Containing type is a controller
+        }
+        static void StaticLocalFunction(HttpRequest request)
+        {
+            _ = request.Form["id"]; // Noncompliant: Containing type is a controller
+        }
+    }
+}
+
+// parameterized test: Repeat for Controller, ControllerBase, MyBaseController, MyBaseBaseController base classes
+public class MyBaseController : ControllerBase { }
+public class MyBaseBaseController : MyBaseController { }
+public class MyTestController : MyBaseBaseController
+{
+    public void Action()
+    {
+        _ = Request.Form["id"]; // Noncompliant
+    }
+}
+
+static class HttpRequestExtensions
+{
+    // parameterized test: parameters are the different forbidden Request accesses (see above)
+    public static void Ext(this HttpRequest request)
+    {
+        _ = request.Form["id"]; // Compliant: Not in a controller
+    }
+}
+
+class RequestService
+{
+    public HttpRequest Request { get; }
+    // parameterized test: parameters are the different forbidden Request accesses (see above)
+    public void HandleRequest(HttpRequest request)
+    {
+        _ = Request.Form["id"]; // Compliant: Not in a controller
+    }
+}

rules/S6932/csharp/metadata.json

@@ -0,0 +1,26 @@
+{
+  "title": "Use model binding instead of reading raw request data",
+  "type": "CODE_SMELL",
+  "status": "ready",
+  "remediation": {
+    "func": "Constant\/Issue",
+    "constantCost": "5min"
+  },
+  "tags": [
+    "asp.net"
+  ],
+  "defaultSeverity": "Major",
+  "ruleSpecification": "RSPEC-6932",
+  "sqKey": "S6932",
+  "scope": "Main",
+  "defaultQualityProfiles": ["Sonar way"],
+  "quickfix": "infeasible",
+  "code": {
+    "impacts": {
+      "MAINTAINABILITY": "HIGH",
+      "RELIABILITY": "MEDIUM",
+      "SECURITY": "MEDIUM"
+    },
+    "attribute": "CONVENTIONAL"
+  }
+}

rules/S6932/csharp/rule.adoc

@@ -0,0 +1,129 @@
+The `HttpRequest` class provides access to the raw request data through the `QueryString`, `Headers`, and `Forms` properties. However, it is recommended to use model binding instead of directly accessing the input data.
+
+== Why is this an issue?
+
+Both ASP.Net MVC implementations - https://learn.microsoft.com/en-us/aspnet/core[Core] and https://learn.microsoft.com/en-us/aspnet/overview[Framework] - support model binding in a comparable fashion. Model binding streamlines the process by automatically aligning data from HTTP requests with action method parameters, providing numerous benefits compared to manually parsing raw incoming request data:
+
+Simplicity:: Model binding simplifies the code by automatically mapping data from HTTP requests to action method parameters. You don't need to write any code to manually extract values from the request.
+Type Safety:: Model binding provides type safety by automatically converting the incoming data into the appropriate .NET types. If the conversion fails, the model state becomes invalid, which you can easily check using `ModelState.IsValid`.
+Validation:: With model binding, you can easily apply validation rules to your models using data annotations. If the incoming data doesn't comply with these rules, the model state becomes invalid.
+Security:: Model binding helps protect against over-posting attacks by only including properties in the model that you explicitly bind using the `[Bind]` attribute or by using view models that only contain the properties you want to update.
+Maintainability:: By using model binding, your code becomes cleaner, easier to read, and maintain. It promotes the use of strongly typed views, which can provide compile-time checking of your views.
+
+== How to fix it in ASP.NET Core
+
+=== Code examples
+
+==== Noncompliant code example
+
+[source,csharp,diff-id=1,diff-type=noncompliant]
+----
+public IActionResult Post()
+{
+    var name = Request.Form["name"];                           // Noncompliant: Request.Form
+    var birthdate = DateTime.Parse(Request.Form["Birthdate"]); // Noncompliant: Request.Form
+
+    var origin = Request.Headers[HeaderNames.Origin];          // Noncompliant: Request.Headers
+    var locale = Request.Query.TryGetValue("locale", out var locales) 
+        ? locales.ToString()
+        : "en-US";                                             // Noncompliant: Request.Query
+    // ..
+}
+----
+
+==== Compliant solution
+
+[source,csharp,diff-id=1,diff-type=compliant]
+----
+public record User
+{
+    [Required, StringLength(100)]
+    public required string Name { get; init; }
+    [DataType(DataType.Date)]
+    public DateTime? Birthdate { get; init; }
+}
+
+public IActionResult Post(User user, [FromHeader] string origin, [FromQuery] string locale = "en-US")
+{
+    if (ModelState.IsValid)
+    {
+        // ...
+    }
+}
+----
+
+== How to fix it in ASP.NET MVC 4.x
+
+=== Code examples
+
+==== Noncompliant code example
+
+[source,csharp,diff-id=2,diff-type=noncompliant]
+----
+public ActionResult Post()
+{
+    var name = Request.Form["name"];                           // Noncompliant: Request.Form
+    var birthdate = DateTime.Parse(Request.Form["Birthdate"]); // Noncompliant: Request.Form
+
+    var locale = Request.QueryString["locale"] ?? "en-US";     // Noncompliant: Request.QueryString
+    // ..
+}
+----
+
+==== Compliant solution
+
+[source,csharp,diff-id=2,diff-type=compliant]
+----
+public class User
+{
+    [Required, StringLength(100)]
+    public string Name { get; set; }
+    [DataType(DataType.Date)]
+    public DateTime? Birthdate { get; set; }
+}
+
+public ActionResult Post(User user, [FromUri] string locale = "en-US")
+{
+    if (ModelState.IsValid)
+    {
+        // ...
+    }
+}
+----
+
+=== How does this work?
+
+Model binding in ASP.NET Core MVC and ASP.NET MVC 4.x works by automatically mapping data from HTTP requests to action method parameters. Here's a step-by-step breakdown of how it works:
+
+1. **Request Data** When a user submits a form or sends a request to an ASP.NET application, the request data might include form data, query string parameters, request body, and HTTP headers.
+
+2. **Model Binder** The model binder's job is to create .NET objects from the request data. It looks at each parameter in the action method and attempts to populate it with the incoming data.
+
+3. **Value Providers** The model binder uses Value Providers to get data from various parts of the request, such as the query string, form data, or route data. Each value provider tells the model binder where to find values in the request.
+
+4. **Binding** The model binder tries to match the keys from the incoming data with the properties of the action method's parameters. If a match is found, it attempts to convert the incoming data into the appropriate .NET type and assigns it to the parameter.
+
+5. **Validation** If the model binder can't convert the value or if the converted value doesn't pass any specified validation rules, it adds an error to the `ModelState.Errors` collection. You can check `ModelState.IsValid` in your action method to see if any errors occurred during model binding.
+
+6. **Action Method Execution** The action method is executed with the bound parameters. If `ModelState.IsValid` is `false`, you can handle the errors in your action method and return an appropriate response.
+
+See the links in the <<Resources>> section for more information.
+
+== Resources
+
+=== Documentation
+
+* Microsoft Learn - ASP.NET MVC 4.x - https://learn.microsoft.com/en-us/aspnet/web-api/overview/formats-and-model-binding/parameter-binding-in-aspnet-web-api[Parameter Binding in ASP.NET Web API]
+* Microsoft Learn - ASP.NET MVC 4.x - https://learn.microsoft.com/en-us/aspnet/mvc/overview/getting-started/introduction/adding-a-controller[Adding a New Controller]
+* Microsoft Learn - ASP.NET MVC 4.x - https://learn.microsoft.com/en-us/aspnet/mvc/overview/getting-started/introduction/adding-a-model[Adding a New Model]
+* Microsoft Learn - ASP.NET MVC 4.x - https://learn.microsoft.com/en-us/aspnet/mvc/overview/getting-started/introduction/adding-validation[Adding Validation]
+* Microsoft Learn - ASP.NET MVC 4.x - https://learn.microsoft.com/en-us/aspnet/web-api/overview/formats-and-model-binding/model-validation-in-aspnet-web-api[Model Validation in ASP.NET Web API]
+* Microsoft Learn - ASP.NET MVC 4.x - https://learn.microsoft.com/en-us/dotnet/api/system.web.httprequest.form[HttpRequest.Form Property]
+* Microsoft Learn - ASP.NET MVC 4.x - https://learn.microsoft.com/en-us/dotnet/api/system.web.httprequest.querystring[HttpRequest.QueryString Property]
+* Microsoft Learn - Asp.Net Core - https://learn.microsoft.com/en-us/aspnet/core/mvc/models/model-binding[Model Binding in ASP.NET Core]
+* Microsoft Learn - Asp.Net Core - https://learn.microsoft.com/en-us/aspnet/core/mvc/models/validation[Model validation in ASP.NET Core MVC and Razor Pages]
+* Microsoft Learn - Asp.Net Core - https://learn.microsoft.com/en-us/aspnet/core/mvc/advanced/custom-model-binding[Custom Model Binding in ASP.NET Core]
+* Microsoft Learn - Asp.Net Core - https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.httprequest.form[HttpRequest.Form Property]
+* Microsoft Learn - Asp.Net Core - https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.httprequest.headers[HttpRequest.Headers Property]
+* Microsoft Learn - Asp.Net Core - https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.httprequest.query[HttpRequest.Query Property]
+* Microsoft Learn - Asp.Net Core - https://learn.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.http.httprequest.routevalues[HttpRequest.RouteValues Property]

rules/S6932/metadata.json

@@ -0,0 +1,2 @@
+{
+}