Add request-scanning middleware, harden filter, rebrand to SpitFire

.github/workflows/dotnet-publish.yml

@@ -1,21 +1,38 @@
-# This workflow will build a .NET project
-# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-net
+# This workflow builds and publishes the NuGet package using Trusted Publishing
+# (OIDC), so no long-lived API key is stored in the repo.
+# See: https://learn.microsoft.com/en-us/nuget/nuget-org/trusted-publishing
+#
+# One-time setup on nuget.org (Account -> Trusted Publishing): add a policy with
+#   Repository Owner: bsaranga
+#   Repository:       PayloadInjectionFilter
+#   Workflow File:    dotnet-publish.yml
+#   Environment:      release   (optional, but recommended; must match the
+#                                job-level `environment: release` below)
+# and add a repository secret NUGET_USER set to your nuget.org username (the
+# profile name, NOT your email address). The `release` environment can also
+# be configured in GitHub (Settings -> Environments) with required reviewers
+# so a publish must be manually approved.
 
 name: Nuget Package Publish Workflow
 
 on:
-  pull_request:
+  push:
     branches: [ "main" ]
 
 jobs:
-  build:
+  build-and-publish:
 
     runs-on: ubuntu-latest
+    environment: release
+
+    permissions:
+      id-token: write   # required for GitHub to issue the OIDC token
+      contents: read
 
     steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
     - name: Setup .NET
-      uses: actions/setup-dotnet@v3
+      uses: actions/setup-dotnet@v4
       with:
         dotnet-version: 6.0.x
     - name: Restore dependencies
@@ -25,6 +42,11 @@ jobs:
     - name: Pack
       run: dotnet pack -o . -c Release
       working-directory: ./PayloadInjectionFilter
+    - name: NuGet login (OIDC -> short-lived API key)
+      uses: NuGet/login@v1
+      id: login
+      with:
+        user: ${{ secrets.NUGET_USER }}
     - name: Publish
-      run: dotnet nuget push *.nupkg --api-key ${{ secrets.NUGET_API_KEY }} --source https://api.nuget.org/v3/index.json
+      run: dotnet nuget push *.nupkg --api-key ${{ steps.login.outputs.NUGET_API_KEY }} --source https://api.nuget.org/v3/index.json --skip-duplicate
       working-directory: ./PayloadInjectionFilter

PayloadInjectionFilter/HelperExtensions.cs

@@ -1,7 +1,7 @@
 using System.Reflection;
 using Microsoft.AspNetCore.Mvc.Filters;
 
-namespace Zone24x7PayloadExtensionFilter.HelperExtensions
+namespace SpitFirePayloadExtensionFilter.HelperExtensions
 {
     public static class HelperExtensions
     {

PayloadInjectionFilter/PayloadInjectionFilter.cs

@@ -5,10 +5,10 @@
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.Extensions.Options;
 using System.Collections;
-using Zone24x7PayloadExtensionFilter.HelperExtensions;
+using SpitFirePayloadExtensionFilter.HelperExtensions;
 using System.Net;
 
-namespace Zone24x7PayloadExtensionFilter
+namespace SpitFirePayloadExtensionFilter
 {
     /// <summary>
     /// This filter intercepts the requests before it
@@ -29,6 +29,13 @@ public class PayloadInjectionFilter : IActionFilter
         private List<string> CaughtMaliciousContent;
         private ActionExecutingContext CurrentContext;
 
+        /// <summary>
+        /// Tracks the reference-type instances on the current traversal path so that
+        /// cyclic object graphs (e.g. A -> B -> A) do not cause infinite recursion.
+        /// Reference equality is used so that distinct sibling instances are not skipped.
+        /// </summary>
+        private readonly HashSet<object> VisitedOnPath = new HashSet<object>(ReferenceEqualityComparer.Instance);
+
         private readonly ILogger<PayloadInjectionFilter> logger;
         private readonly IOptions<PayloadInjectionOptions> options;
 
@@ -89,9 +96,14 @@ public void OnActionExecuting(ActionExecutingContext context)
 
                 if (hasWhiteListedEntries)
                 {
-                    pathTemplate = context.ActionDescriptor.AttributeRouteInfo.Template;
-                    templateMatch = options.Value.WhiteListEntries.Select(w => w.PathTemplate).Contains(pathTemplate);
-                    whiteListIndex = (templateMatch) ? options.Value.WhiteListEntries.Select(w => w.PathTemplate).ToList().IndexOf(pathTemplate) : -1;
+                    // AttributeRouteInfo is null for convention-routed controllers; guard against it.
+                    pathTemplate = context.ActionDescriptor.AttributeRouteInfo?.Template;
+
+                    if (pathTemplate != null)
+                    {
+                        templateMatch = options.Value.WhiteListEntries.Select(w => w.PathTemplate).Contains(pathTemplate);
+                        whiteListIndex = (templateMatch) ? options.Value.WhiteListEntries.Select(w => w.PathTemplate).ToList().IndexOf(pathTemplate) : -1;
+                    }
                 }
 
                 if (context.IsOneOfAllowedHttpMethods(options.Value.AllowedHttpMethods!.Select(x => x.ToString()).Distinct().ToArray()))
@@ -100,12 +112,15 @@ public void OnActionExecuting(ActionExecutingContext context)
 
                     foreach (var argument in context.ActionArguments)
                     {
+                        // A nullable/optional bound parameter can be null; skip it instead of throwing.
+                        if (argument.Value == null) continue;
+
                         if (hasWhiteListedEntries && whiteListIndex != -1)
                         {
                             parameterMatch = options.Value.WhiteListEntries[whiteListIndex].ParameterName.Equals(argument.Key);
                             whiteListInitialCondition = parameterMatch && templateMatch;
                         }
-                        
+
                         var argumentType = argument.Value.GetType();
 
                         if (argumentType.IsString())
@@ -118,6 +133,7 @@ public void OnActionExecuting(ActionExecutingContext context)
                         {
                             foreach (var listItem in (argument.Value as IEnumerable)!)
                             {
+                                if (listItem == null) continue;
                                 Evaluate(listItem.GetType(), listItem, context, whiteListIndex, whiteListInitialCondition, ref PLACE_HOLDER_RECURSION_DEPTH);
                             }
                         }
@@ -142,40 +158,75 @@ private void Evaluate(Type argType, object arg, ActionExecutingContext context,
             {
                 if (arg != null && (!argType.IsValueType || argType.IsKeyValuePair()))
                 {
-                    IEnumerable<PropertyInfo> properties = argType.GetProperties(BindingFlags.Public | BindingFlags.Instance);
+                    // Cycle guard: if this exact instance is already on the current traversal
+                    // path, recursing again would loop forever. Track it for the duration of
+                    // this subtree and remove it on the way out so siblings are still scanned.
+                    bool tracked = !argType.IsValueType && VisitedOnPath.Add(arg);
+                    if (!tracked && !argType.IsValueType) return;
 
-                    if (properties.Any())
+                    try
                     {
+                        PropertyInfo[] properties = GetCachedProperties(argType);
+
                         foreach (var prop in properties)
                         {
                             isWhiteListedProperty = whiteListIndex != -1 && options.Value.WhiteListEntries[whiteListIndex].PropertyNames.Contains(prop.Name);
 
                             if (prop.IsString() && !(initialWhiteListCondition && isWhiteListedProperty))
                             {
-                                if (DetectDisallowedChars(prop.GetValue(arg) as string, options.Value.Pattern ?? DEFAULT_FILTER_PATTERN))
+                                var value = prop.GetValue(arg) as string;
+                                if (DetectDisallowedChars(value, options.Value.Pattern ?? DEFAULT_FILTER_PATTERN))
+                                {
+                                    ShortCircuit(context, value);
+                                }
+                            }
+                            else if (prop.IsString() && initialWhiteListCondition && isWhiteListedProperty
+                                     && whiteListIndex != -1 && options.Value.WhiteListEntries[whiteListIndex].ExclusionPattern != null)
+                            {
+                                // The property is white-listed, but an ExclusionPattern is configured:
+                                // still short-circuit if the (otherwise allowed) value matches it.
+                                var value = prop.GetValue(arg) as string;
+                                if (DetectDisallowedChars(value, options.Value.WhiteListEntries[whiteListIndex].ExclusionPattern))
                                 {
-                                    ShortCircuit(context, prop.GetValue(arg) as string);
+                                    ShortCircuit(context, value);
                                 }
                             }
                             else if (!prop.PropertyType.IsValueType)
                             {
+                                var value = prop.GetValue(arg);
+                                if (value == null) continue;
+
                                 if (prop.PropertyType.IsEnumerable())
                                 {
-                                    if (prop.GetValue(arg) != null)
+                                    foreach (var item in (value as IEnumerable))
                                     {
-                                        foreach (var item in (prop.GetValue(arg) as IEnumerable))
-                                            Evaluate(item.GetType(), item, context, whiteListIndex, initialWhiteListCondition, ref CurrentRecursionDepth);
+                                        if (item == null) continue;
+                                        Evaluate(item.GetType(), item, context, whiteListIndex, initialWhiteListCondition, ref CurrentRecursionDepth);
                                     }
                                 }
-                                else Evaluate(prop.PropertyType, prop.GetValue(arg), context, whiteListIndex, initialWhiteListCondition, ref CurrentRecursionDepth);
+                                else Evaluate(prop.PropertyType, value, context, whiteListIndex, initialWhiteListCondition, ref CurrentRecursionDepth);
                             }
                         }
                     }
+                    finally
+                    {
+                        if (tracked) VisitedOnPath.Remove(arg);
+                    }
                 }
             }
             else RecursionDepthExceeded(context);
         }
 
+        /// <summary>
+        /// Caches the public instance properties per <see cref="Type"/> so the reflection
+        /// metadata lookup is performed only once per type rather than on every request.
+        /// </summary>
+        private static readonly System.Collections.Concurrent.ConcurrentDictionary<Type, PropertyInfo[]> PropertyCache
+            = new System.Collections.Concurrent.ConcurrentDictionary<Type, PropertyInfo[]>();
+
+        private static PropertyInfo[] GetCachedProperties(Type type)
+            => PropertyCache.GetOrAdd(type, t => t.GetProperties(BindingFlags.Public | BindingFlags.Instance));
+
         private bool DetectDisallowedChars(string input, Regex disallowedPattern)
         {
             if (string.IsNullOrEmpty(input)) return false;

PayloadInjectionFilter/PayloadInjectionFilter.csproj

@@ -3,10 +3,10 @@
   <PropertyGroup>
     <TargetFramework>net6.0</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
-	<PackageId>Zone24x7.PayloadInjectionFilter</PackageId>
-	<Version>0.0.9</Version>
+	<PackageId>SpitFire.PayloadInjectionFilter</PackageId>
+	<Version>0.1.0</Version>
 	<Authors>Buwaneka Saranga</Authors>
-	<Company>Zone24x7 Inc.</Company>
+	<Company>SpitFire Inc.</Company>
 	<Product>PayloadInjectionFilter For ASPNET Core</Product>
 	<Description>
 		A reusable payload injection filter for ASP.NET Core. This can be used to short-circuit a request if it contains any malicious content as specified via a regex pattern.

PayloadInjectionFilter/PayloadInjectionFilterConfigurationExtensions.cs

@@ -1,6 +1,6 @@
 using Microsoft.Extensions.DependencyInjection;
 
-namespace Zone24x7PayloadExtensionFilter
+namespace SpitFirePayloadExtensionFilter
 {
     /// <summary>
     /// Contains payload injection filter extensible methods

PayloadInjectionFilter/PayloadInjectionMiddleware.cs

@@ -0,0 +1,132 @@
+using System.Net;
+using System.Text;
+using System.Text.RegularExpressions;
+using Microsoft.AspNetCore.Http;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
+
+namespace SpitFirePayloadExtensionFilter
+{
+    /// <summary>
+    /// Middleware that scans the raw request (query string and body) for disallowed content
+    /// before it reaches the endpoint pipeline. Because it runs before model binding it covers
+    /// MVC controllers, minimal APIs, Razor Pages and gRPC alike.
+    /// <para>
+    /// This is a defense-in-depth boundary check, not a substitute for output encoding,
+    /// parameterized queries or a dedicated WAF. Deny-listing characters such as
+    /// <c>&lt; &gt; &amp; ;</c> will reject some legitimate input — exclude such routes via
+    /// <see cref="PayloadInjectionMiddlewareOptions.ExcludedPaths"/>.
+    /// </para>
+    /// </summary>
+    public class PayloadInjectionMiddleware
+    {
+        private readonly RequestDelegate next;
+        private readonly ILogger<PayloadInjectionMiddleware> logger;
+        private readonly PayloadInjectionMiddlewareOptions options;
+        private readonly HashSet<string> methods;
+        private readonly Regex pattern;
+
+        public PayloadInjectionMiddleware(
+            RequestDelegate next,
+            IOptions<PayloadInjectionMiddlewareOptions> options,
+            ILogger<PayloadInjectionMiddleware> logger)
+        {
+            this.next = next;
+            this.logger = logger;
+            this.options = options.Value;
+            this.pattern = this.options.Pattern ?? new Regex(@"[<>\&;]");
+            this.methods = new HashSet<string>(
+                (this.options.AllowedHttpMethods ?? new List<HttpMethod>()).Select(m => m.Method),
+                StringComparer.OrdinalIgnoreCase);
+        }
+
+        public async Task InvokeAsync(HttpContext context)
+        {
+            var request = context.Request;
+
+            if (!methods.Contains(request.Method) || IsExcluded(request.Path))
+            {
+                await next(context);
+                return;
+            }
+
+            if (options.ScanQueryString && request.QueryString.HasValue)
+            {
+                // Decode so percent-encoded payloads (e.g. %3Cscript%3E) are caught too.
+                var query = Uri.UnescapeDataString(request.QueryString.Value!);
+                if (pattern.IsMatch(query))
+                {
+                    await ShortCircuitAsync(context, "query string");
+                    return;
+                }
+            }
+
+            if (options.ScanBody && HasBody(request))
+            {
+                if (request.ContentLength.HasValue && request.ContentLength.Value > options.MaxScannedBodyBytes)
+                {
+                    await RejectOversizedAsync(context);
+                    return;
+                }
+
+                var body = await ReadBodyAsync(request);
+                if (body != null && pattern.IsMatch(body))
+                {
+                    await ShortCircuitAsync(context, "request body");
+                    return;
+                }
+            }
+
+            await next(context);
+        }
+
+        private bool IsExcluded(PathString path)
+        {
+            if (options.ExcludedPaths == null || options.ExcludedPaths.Count == 0) return false;
+            return options.ExcludedPaths.Any(p => path.StartsWithSegments(p, StringComparison.OrdinalIgnoreCase));
+        }
+
+        private static bool HasBody(HttpRequest request)
+            => (request.ContentLength ?? 0) > 0 || request.Headers.ContainsKey("Transfer-Encoding");
+
+        private async Task<string> ReadBodyAsync(HttpRequest request)
+        {
+            // Buffer the body so it can be re-read by the endpoint after scanning.
+            request.EnableBuffering();
+
+            using var reader = new StreamReader(
+                request.Body,
+                Encoding.UTF8,
+                detectEncodingFromByteOrderMarks: false,
+                bufferSize: 1024,
+                leaveOpen: true);
+
+            var body = await reader.ReadToEndAsync();
+            request.Body.Position = 0;
+            return body;
+        }
+
+        private async Task ShortCircuitAsync(HttpContext context, string location)
+        {
+            logger.LogWarning(
+                "[PayloadInjectionMiddleware]:[Warning] Request short-circuited due to malicious content in the {Location}.",
+                location);
+
+            context.Response.Clear();
+            context.Response.StatusCode = options.ResponseStatusCode == 0 ? 400 : options.ResponseStatusCode;
+            context.Response.ContentType = options.ResponseContentType ?? "text/plain";
+            await context.Response.WriteAsync(
+                options.ResponseContentBody ?? "Request short-circuited due to malicious content.");
+        }
+
+        private async Task RejectOversizedAsync(HttpContext context)
+        {
+            logger.LogWarning("[PayloadInjectionMiddleware]:[Warning] Request body exceeds the scannable limit of {Limit} bytes.", options.MaxScannedBodyBytes);
+
+            context.Response.Clear();
+            context.Response.StatusCode = (int)HttpStatusCode.RequestEntityTooLarge;
+            context.Response.ContentType = "text/plain";
+            await context.Response.WriteAsync("Request body is too large to scan.");
+        }
+    }
+}