Skip to content

Commit

Permalink
Modify S3355(xml): Migrate to LayC (#3370)
Browse files Browse the repository at this point in the history
## 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>
  • Loading branch information
1 parent 3ba09d2 commit 1ad5902
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 42 deletions.
5 changes: 3 additions & 2 deletions rules/S3355/xml/metadata.json
Original file line number Diff line number Diff line change
@@ -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"
},
Expand Down
104 changes: 64 additions & 40 deletions rules/S3355/xml/rule.adoc
Original file line number Diff line number Diff line change
@@ -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 ``++<filter-mapping>++`` 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]
----
<filter>
<filter-name>DefinedNotUsed</filter-name>
<filter-class>com.myco.servlet.ValidationFilter</filter-class>
</filter>
----
If a filter is not used in a ``++<filter-mapping>++`` 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]
----
<filter>
<filter-name>ValidationFilter</filter-name>
<filter-class>com.myco.servlet.ValidationFilter</filter-class>
</filter>
<filter-mapping>
<filter-name>ValidationFilter</filter-name>
<url-pattern>/*</url-pattern>
</filter-mapping>
<filter>
<filter-name>ValidationFilter</filter-name> <!-- Noncompliant -->
<filter-class>com.myco.servlet.ValidationFilter</filter-class>
</filter>
----

==== Compliant solution

[source,xml,diff-id=1,diff-type=compliant]
----
<filter>
<filter-name>ValidationFilter</filter-name>
<filter-class>com.myco.servlet.ValidationFilter</filter-class>
</filter>
<filter-mapping>
<filter-name>ValidationFilter</filter-name>
<url-pattern>/*</url-pattern>
</filter-mapping>
----

== 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[]
Expand All @@ -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[]

0 comments on commit 1ad5902

Please sign in to comment.