Merge pull request #403 from Lombiq/issue/NEST-536

NEST-536: Add necessary security exceptions
Lombiq · Aug 23, 2024 · f9ecdfd · f9ecdfd
2 parents e40df43 + 387ae20
commit f9ecdfd
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 9 deletions.
diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml
@@ -4,7 +4,10 @@ env:
     - name: Default Context
       urls:
         - <start URL>
-      excludePaths: []
+      excludePaths:
+        # Don't inspect any part of the Admin dashboard or the testing-exclusive Lombiq.Tests.UI.Shortcuts module.
+        - .*/Admin/.*
+        - .*/Lombiq.Tests.UI.Shortcuts/.*
       authentication:
         parameters: {}
         verification:
@@ -66,6 +69,34 @@ jobs:
     name: passiveScan-config
     type: passiveScan-config
   - alertFilters:
+      # Setting the ReturnUrl query parameter to itself yields a false positive.
+      - ruleId: 6
+        ruleName: Path Traversal (6)
+        context: ''
+        newRisk: False Positive
+        parameter: ''
+        parameterRegex: false
+        url: .*/(Login|ChangePassword)[?][rR]eturnUrl=.*
+        urlRegex: true
+        attack: ''
+        attackRegex: false
+        evidence: ''
+        evidenceRegex: false
+        methods: []
+      # Setting the ReturnUrl query parameter to an SQL expression can't actually cause SQL Injection.
+      - ruleId: 40018
+        ruleName: SQL Injection (40018)
+        context: ''
+        newRisk: False Positive
+        parameter: ''
+        parameterRegex: false
+        url: .*/(Login|ChangePassword)[?][rR]eturnUrl=.*
+        urlRegex: true
+        attack: ''
+        attackRegex: false
+        evidence: ''
+        evidenceRegex: false
+        methods: []
       # Mistakes a system-property('xsl:vendor') XSLT injection attempt as successful due to Microsoft being there on
       # the login screen at all times for External Login. Might happen in similar cases with other brand names too.
       - ruleId: 90017
@@ -74,7 +105,7 @@ jobs:
         newRisk: False Positive
         parameter: ''
         parameterRegex: false
-        url: .*/(ChangePassword|Account/LinkLogin|Account/ExternalLogin|Users/LogOff).*
+        url: .*/(ChangePassword|Account/LinkLogin|Account/ExternalLogin|Users/LogOff|api/content).*
         urlRegex: true
         attack: ''
         attackRegex: false

diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml
@@ -62,6 +62,26 @@ jobs:
         threshold: high
     name: passiveScan-config
     type: passiveScan-config
+  - alertFilters:
+      # Mistakes a system-property('xsl:vendor') XSLT injection attempt as successful due to Microsoft being there on
+      # the login screen at all times for External Login. Might happen in similar cases with other brand names too.
+      - ruleId: 90017
+        ruleName: XSLT Injection (90017)
+        context: ''
+        newRisk: False Positive
+        parameter: ''
+        parameterRegex: false
+        url: .*/(api/content).*
+        urlRegex: true
+        attack: ''
+        attackRegex: false
+        evidence: ''
+        evidenceRegex: false
+        methods: []
+    parameters:
+      deleteGlobalAlerts: false
+    name: alertFilter
+    type: alertFilter
   - parameters: {}
     name: openapi
     type: openapi

diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs
@@ -82,9 +82,6 @@ public static Task RunAndConfigureAndAssertFullSecurityScanForContinuousIntegrat
                 // Signing in ensures full access and that the bot won't have to interact with the login screen.
                 if (doSignIn) configuration.SignIn();
 
-                // There is no need to security scan the admin dashboard.
-                configuration.ExcludeUrlWithRegex(@".*/Admin/.*");
-
                 // Active scan takes a very long time, this is not practical in CI.
                 configuration.ModifyZapPlan(plan => plan
                     .SetActiveScanMaxDuration(maxActiveScanDurationInMinutes, maxRuleDurationInMinutes));

diff --git a/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs b/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs
@@ -59,10 +59,7 @@ public class OrchardCoreUITestExecutorConfiguration
             !logEntry.Message.ContainsOrdinalIgnoreCase("HTML Imports is deprecated") &&
             // The 404 is because of how browsers automatically request /favicon.ico even if a favicon is declared to be
             // under a different URL.
-            !logEntry.IsNotFoundLogEntry("/favicon.ico") &&
-            // Workaround for https://github.com/OrchardCMS/OrchardCore/issues/16606.
-            !(logEntry.Message.ContainsOrdinalIgnoreCase("/Settings/general") &&
-                logEntry.Message.ContainsOrdinalIgnoreCase("A em tag was parsed inside of a <select> which was not inserted into the document."));
+            !logEntry.IsNotFoundLogEntry("/favicon.ico");
 
     /// <summary>
     /// Gets the global events available during UI test execution.