Patch Rewards Program submission for apache/struts#781

PiperOrigin-RevId: 704484456

Author: Googler

patch-rewards-program/rewarded-patches/apache/struts/330511857.md

@@ -0,0 +1,118 @@
+---
+
+### Improvement category
+
+
+#### Submission title
+
+Intelligent allowlist based sandbox for OGNL evaluations within the Struts web framework
+
+
+#### Please select the best-matching category:
+
+Improvement to privilege separation or sandboxing
+
+
+---
+
+### Patch details
+
+
+#### Specify project (the scope is limited to the listed projects):
+
+Popular web frameworks and libraries: Angular, Closure, Dart, Django, Dojo Foundation, Ember, GWT, Go, Jinja (Werkzeug, Flask), jQuery, Knockout, Polymer, Struts, Web2py, Wicket
+
+
+#### Project name
+
+Apache Struts
+
+
+#### Patch name or short description
+
+Intelligent allowlist based sandbox for OGNL evaluations within the Struts web framework
+
+
+#### Links to the diffs of the patch
+
+https://github.com/apache/struts/pull/781 https://github.com/apache/struts/pull/791 https://github.com/apache/struts/pull/800 https://github.com/apache/struts/pull/826 https://github.com/apache/struts/pull/831 https://github.com/apache/struts/pull/832
+
+
+#### Complexity – select the option that best describes the patch's complexity:
+
+High effort or complexity
+
+
+#### Impact – select the option that best describes the security impact of the patch:
+
+Almost certain to prevent major vulnerabilities in the affected code
+
+
+#### Tell us more about the patch
+
+Apache Struts has historically been plagued by the most critical vulnerabilities, often remote code executions. This is due to its use of OGNL, a powerful expression language, which enables many of its key features. Due to OGNL’s capabilities, if a bad actor is able to achieve arbitrary expression injection, it is often trivial to escalate to an RCE. Some previous framework design decisions also mean that exploitation paths are often introduced inadvertently by Struts-based web developers not familiar with the Struts framework’s inner workings.
+
+OGNL expression injection based vulnerabilities in Struts have usually been achieved in 2 ways:
+1. By leveraging Struts’ parameter deserialization mechanism. This is an intended OGNL expression injection point for users, whereby DTO classes are populated with user/form data. The OGNL expressions used for this purpose are sanitised using a RegEx which limits the OGNL syntax capabilities. However, these expressions still prove dangerous as will be explained later.
+2. By exploiting a potential SSTI vulnerability (sometimes known as a double evaluation). In such a scenario, no RegEx based sanitisation takes place as internal OGNL evaluations are intended to leverage all OGNL syntax capabilities.
+
+This patch submission addresses serious security concerns with both the parameter deserializing **intended** OGNL expression injection point, as well as any **unintended** injection points.
+
+Struts already had a class exclusion list mechanism for evaluating OGNL expressions. Whilst this exclusion list contained the most obvious classes that could be used maliciously, it required proactive application-specific maintenance to ensure any sensitive application beans or library classes capable of privileged operations were included on this list. Often it might not be apparent that a certain class could pose a threat as an exploit gadget until an exploit had already occurred.
+
+The most obvious solution here was to introduce a class allow list in place of the exclusion list. However, obtaining a list of all classes needed on such an allow list would prove an equally rigorous endeavour, and the aim here was to have this capability enabled by default for all Struts applications and minimise maintenance burden. Most Struts applications loaded their configuration from an XML file. By having a pre-populated allow list consisting of necessary Struts library classes and then supplementing it with relevant classes while loading a specific application’s XML configuration, the vast majority of classes needed for a fully functional Struts application could be achieved.
+
+For most Struts applications, the only remaining classes that were missing
+from the allow list were their form submission DTO classes. Struts has the
+concept of Action classes which contains the business logic for serving and
+handling web pages, as well as any fields or nested classes to where form
+submission parameters can be deserialized. Struts' parameter
+deserialization mechanism can be used to set user-controlled data on ANY
+public member on the Action class. Given business logic cohabits this
+class, it becomes painfully easy to accidentally expose a sensitive
+application bean which is then completely open to user mutation.
+
+Struts has never required the explicit declaration of methods/fields
+intended for parameter deserialization and thereby clearly separating them
+from other methods/fields in the Action class. There was previously an
+attempt to introduce such a mechanism
+(AnnotationParameterFilterInterceptor) but it was entirely optional, did
+not block parameters by default, and its field-based annotation mechanism
+was ineffective for many exploitable scenarios (such as a getter returning
+an object that doesn't correspond to a field).
+
+Thus, in this patch I additionally introduce a method-based annotation
+which is used to explicitly declare parameter deserialization points,
+clearly marking the invocation path that OGNL takes. Any user attempt at
+setting a parameter value on a member not intended for deserialization will
+be discarded. This annotation mechanism synergises with the earlier
+introduced allow list. Using the annotations I could detect and load the
+final classes needed in the allow list, making it completely
+configuration-less for most Struts application developers. Even if OGNL
+expression injection via an SSTI is achieved (where most OGNL syntax
+capabilities are available to an attacker), there are now a very limited
+number of classes that can be invoked, if an access path even exists.
+
+Implementing the overall patch required the use of numerous creative techniques and a deep understanding of the inner workings of both Struts and OGNL. I additionally dealt with complexities related to bean injection, as managed by the pre-release version of Guice bundled with Struts, and rectifying many existing bugs and suboptimal patterns. Please refer to the linked PRs and full diffs for further details.
+
+In recent months, Atlassian’s Confluence Data Center, which is built upon Apache Struts, was subject to multiple Critical and High rated vulnerabilities. These were shown to have been completely preventable by the capabilities introduced in this patch. Specific PoCs and detailed explanations on how this patch would have thwarted these CVEs can be provided upon request.
+
+Critical/High CVEs preventable by this patch:
+* CVE-2023-22515
+* CVE-2024-21672
+* CVE-2024-21673
+* CVE-2024-21674
+
+I do believe this patch to be one of the greatest uplifts to Struts’ security posture since its inception. Whilst the Struts framework may be waning in popularity since its peak well over a decade ago, this patch ensures applications built upon Struts can be trusted to experience a much higher level of security. It is already available in test builds and will be officially released in the upcoming Struts 6.4.0 as an opt-in capability, and enabled by default in Struts 7.0.0.
+
+---
+
+### Credit
+
+https://github.com/kusalk
+
+---
+
+### Reward Amount
+
+$10000.00 -- complicated, high-impact improvements that almost certainly prevent major vulnerabilities in the affected code