From 1ad5902a8d9a80295b3fd7a3a648113214ea941a Mon Sep 17 00:00:00 2001 From: Loris S <91723853+loris-s-sonarsource@users.noreply.github.com> Date: Thu, 26 Oct 2023 15:19:47 +0200 Subject: [PATCH] Modify S3355(xml): Migrate to LayC (#3370) ## Review A dedicated reviewer checked the rule description successfully for: - [ ] logical errors and incorrect information - [ ] information gaps and missing content - [ ] text style and tone - [ ] PR summary and labels follow [the guidelines](https://github.com/SonarSource/rspec/#to-modify-an-existing-rule) --------- Co-authored-by: Sebastien Andrivet <138577785+sebastien-andrivet-sonarsource@users.noreply.github.com> --- rules/S3355/xml/metadata.json | 5 +- rules/S3355/xml/rule.adoc | 104 +++++++++++++++++++++------------- 2 files changed, 67 insertions(+), 42 deletions(-) diff --git a/rules/S3355/xml/metadata.json b/rules/S3355/xml/metadata.json index 9c41d095427..73f047188cb 100644 --- a/rules/S3355/xml/metadata.json +++ b/rules/S3355/xml/metadata.json @@ -1,9 +1,10 @@ { - "title": "Defined filters should be used", + "title": "Struts filters should not miss their corresponding filter-map", "type": "VULNERABILITY", "code": { "impacts": { - "SECURITY": "HIGH" + "SECURITY": "HIGH", + "MAINTAINABILITY": "HIGH" }, "attribute": "LOGICAL" }, diff --git a/rules/S3355/xml/rule.adoc b/rules/S3355/xml/rule.adoc index 20d23d43537..722e59edd98 100644 --- a/rules/S3355/xml/rule.adoc +++ b/rules/S3355/xml/rule.adoc @@ -1,40 +1,82 @@ +This vulnerability exposes the application to failures of a wide range of +application-specific features the Strut filter was supposed to perform, such as +authentication, logging, encryption, and more. + == Why is this an issue? -Every filter defined in ``++web.xml++`` file should be used in a ``++++`` element. Otherwise such filters are not invoked. +Filters are used to intercept requests and responses from a server and allow +developers to manipulate them. When a `filter` is declared, but the +corresponding `filter assignment` is inadvertently not, then the code is +vulnerable to security problems or business logic instability. +If a filter is defined in the web application descriptor file `web.xml` but is +not used in a "filter mapping", this is an indication that it may have been +forgotten. -=== Noncompliant code example +=== What is the potential impact? -[source,xml] ----- - - DefinedNotUsed - com.myco.servlet.ValidationFilter - ----- +If a filter is not used in a ``++++`` element, it will not be +called. Below are some examples of the impact of this oversight. + +==== Unauthorized access + +One of the main uses of Struts filters is to provide security measures such as +authentication and authorization. If a filter is forgotten in the filter +mappings, unauthorized users could gain access to sensitive data or perform +actions that they are not authorized to perform. + +==== Functional problems +Filters can also be used to modify requests and responses, format data, or even +handle errors. If these features are not included in the filter mappings, they +may not work as expected, resulting in a poor user experience or even +application crash. -=== Compliant solution +==== Performance issues -[source,xml] +Some filters are designed to improve the performance of your application, such +as those that implement caching strategies. If these are not mapped, you may +experience slow response times or increased server load on your application. + +== How to fix it + +=== Code examples + +==== Noncompliant code example + +[source,xml,diff-id=1,diff-type=noncompliant] ---- - - ValidationFilter - com.myco.servlet.ValidationFilter - - - - ValidationFilter - /* - + + ValidationFilter + com.myco.servlet.ValidationFilter + ---- +==== Compliant solution + +[source,xml,diff-id=1,diff-type=compliant] +---- + + ValidationFilter + com.myco.servlet.ValidationFilter + + + + ValidationFilter + /* + +---- == Resources -* https://owasp.org/Top10/A05_2021-Security_Misconfiguration/[OWASP Top 10 2021 Category A5] - Security Misconfiguration -* https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration[OWASP Top 10 2017 Category A6] - Security Misconfiguration +=== Documentation + +* Struts Docs - https://struts.apache.org/core-developers/web-xml[Web.xml Developpers Guide] +=== Standards + +* OWASP - https://owasp.org/Top10/A05_2021-Security_Misconfiguration/[Top 10 2021 Category A5 - Security Misconfiguration] +* OWASP - https://owasp.org/www-project-top-ten/2017/A6_2017-Security_Misconfiguration[Top 10 2017 Category A6 - Security Misconfiguration] ifdef::env-github,rspecator-view[] @@ -47,24 +89,6 @@ ifdef::env-github,rspecator-view[] * "xxx" filter should have a mapping. - ''' -== Comments And Links -(visible only on this page) - -=== on 2 Oct 2015, 19:13:58 Ann Campbell wrote: -\[~nicolas.peru], I know we're not ready to check ``++web.xml++`` yet, but when we are... - -=== on 4 Mar 2016, 12:32:16 sytze van koningsveld wrote: -scrubbing sounds like blacklisting, which is different from true validation (white listing), so maybe a misnomer. The "validation filter" should not replace true valdation, typically done in controllers. - -=== on 19 Mar 2018, 09:56:33 Sébastien GIORIA - AppSecFR wrote: -I might tag this OWASP A6:2017 and not A1:2017. This is not a injection, more a configuration problem - -=== on 20 Mar 2018, 07:22:40 Freddy Mallet wrote: -I tend to agree with [~SPoint]. [~alexandre.gigleux] would you be confortable with this change ? Thanks - -=== on 20 Mar 2018, 08:16:15 Alexandre Gigleux wrote: -I agree [~freddy.mallet] / [~SPoint] and I applied the suggested change. endif::env-github,rspecator-view[]