From 232c30bcdc04501e2b6c5c7e4713885d89debc88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 22 Aug 2024 15:18:55 +0200 Subject: [PATCH 01/11] Add necessary security exceptions. --- .../SecurityScanningUITestContextExtensions.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index 086aeb301..5508893df 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -85,6 +85,15 @@ public static Task RunAndConfigureAndAssertFullSecurityScanForContinuousIntegrat // There is no need to security scan the admin dashboard. configuration.ExcludeUrlWithRegex(@".*/Admin/.*"); + // There is no need to security scan anything in Lombiq.Tests.UI.Shortcuts. + configuration.ExcludeUrlWithRegex(@".*/Lombiq.Tests.UI.Shortcuts/.*"); + + configuration.MarkScanRuleAsFalsePositiveForUrlWithRegex( + ".*/(Login|ChangePassword)([?].*)?", + 6, + "Path Traversal", + "Setting the returnUrl attribute to a itself yields a false positive"); + // Active scan takes a very long time, this is not practical in CI. configuration.ModifyZapPlan(plan => plan .SetActiveScanMaxDuration(maxActiveScanDurationInMinutes, maxRuleDurationInMinutes)); From a54112ae44d9130ee2c4bee7560f5352110f4753 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 22 Aug 2024 16:06:14 +0200 Subject: [PATCH 02/11] Exclude XSLT Injection (90017) from api/content too. --- .../SecurityScanning/AutomationFrameworkPlans/FullScan.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml index 23d5f8734..caf53fa39 100644 --- a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml +++ b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/FullScan.yml @@ -74,7 +74,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 From 60a870b3483ca40cedc41b05c2ae00fe09dd29f0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 22 Aug 2024 19:27:42 +0200 Subject: [PATCH 03/11] Another exception. --- .../SecurityScanningUITestContextExtensions.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs index 5508893df..883acdfec 100644 --- a/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs +++ b/Lombiq.Tests.UI/SecurityScanning/SecurityScanningUITestContextExtensions.cs @@ -89,10 +89,16 @@ public static Task RunAndConfigureAndAssertFullSecurityScanForContinuousIntegrat configuration.ExcludeUrlWithRegex(@".*/Lombiq.Tests.UI.Shortcuts/.*"); configuration.MarkScanRuleAsFalsePositiveForUrlWithRegex( - ".*/(Login|ChangePassword)([?].*)?", + ".*/(Login|ChangePassword)[?][rR]eturnUrl=.*", 6, "Path Traversal", - "Setting the returnUrl attribute to a itself yields a false positive"); + "Setting the ReturnUrl query parameter to a itself yields a false positive"); + + configuration.MarkScanRuleAsFalsePositiveForUrlWithRegex( + ".*/(Login|ChangePassword)[?][rR]eturnUrl=.*", + 40018, + "SQL Injection", + "Setting the ReturnUrl query parameter to an SQL expression can't actually cause SQL Injection."); // Active scan takes a very long time, this is not practical in CI. configuration.ModifyZapPlan(plan => plan From 9fc082dfeee8216d073f2fa7eee9de6645746ca3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Thu, 22 Aug 2024 20:49:07 +0200 Subject: [PATCH 04/11] Exclude API content from OpenAPI.yml too. --- .../AutomationFrameworkPlans/OpenAPI.yml | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml b/Lombiq.Tests.UI/SecurityScanning/AutomationFrameworkPlans/OpenAPI.yml index aa7b05860..cd6d5ee64 100644 --- 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 From 99297c6915efdb0c3fb168ee044b446e97ccce55 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A1ra=20El-Saig?= Date: Fri, 23 Aug 2024 10:25:16 +0200 Subject: [PATCH 05/11] Revert 0f4944f3b5676bb5c510773baae7bf8681539b78 --- .../Services/OrchardCoreUITestExecutorConfiguration.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs b/Lombiq.Tests.UI/Services/OrchardCoreUITestExecutorConfiguration.cs index 1423dfa04..9fb9534bc 100644 --- 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