Change text to fit LaYC format.

rules/S4684/java/rule.adoc

@@ -1,22 +1,50 @@
+With Spring, when a request mapping method is configured to accept bean objects
+as arguments, the framework will automatically bind HTTP parameters to those
+objects' properties. If the targeted beans are also persistent entities, the
+framework will also store those properties in the storage backend, usually the
+application's database.
+
 == Why is this an issue?
 
-On one side, Spring MVC automatically bind request parameters to beans declared as arguments of methods annotated with ``++@RequestMapping++``. Because of this automatic binding feature, it's possible to feed some unexpected fields on the arguments of the ``++@RequestMapping++`` annotated methods. 
+By accepting persistent entities as method arguments, the application allows
+clients to manipulate the object's properties directly.
+
+=== What is the potential impact?
+
+Attackers could forge malicious HTTP requests that will alter unexpected
+properties of persistent objects. This can lead to unauthorized modifications of
+the entity's state. This is known as a *mass assignment* attack.
 
-On the other end, persistent objects (``++@Entity++`` or ``++@Document++``) are linked to the underlying database and updated automatically by a persistence framework, such as Hibernate, JPA or Spring Data MongoDB.
+Depending on the affected objects and properties, the consequences can vary.
 
+==== Privilege escalation
 
-These two facts combined together can lead to malicious attack: if a persistent object is used as an argument of a method annotated with ``++@RequestMapping++``, it's possible from a specially crafted user input, to change the content of unexpected fields into the database.
+If the affected object is used to store the client's identity or permissions,
+the attacker could alter it to change their entitlement on the application. This
+can lead to horizontal or vertical privilege escalation.
 
+==== Security checks bypass
 
-For this reason, using ``++@Entity++`` or ``++@Document++`` objects as arguments of methods annotated with ``++@RequestMapping++`` should be avoided.
+Because persistent objects are modified directly without prior logic, attackers
+could exploit this issue to bypass security measures otherwise enforced by the
+application. For example, an attacker might be able to change their e-mail
+address to an invalid one by directly setting it without going through the
+application's email validation process.
 
+The same could also apply to passwords that attackers could change without
+complexity validation or knowledge of their current value.
 
-In addition to ``++@RequestMapping++``, this rule also considers the annotations introduced in Spring Framework 4.3: ``++@GetMapping++``, ``++@PostMapping++``, ``++@PutMapping++``, ``++@DeleteMapping++``, ``++@PatchMapping++``.
+== How to fix it in Java EE
 
+=== Code examples
 
-=== Noncompliant code example
+The following code is vulnerable to a mass assignment attack because it allows
+modifying the `User` persistent entities thanks to maliciously forged `Wish`
+object properties.
 
-[source,java]
+==== Noncompliant code example
+
+[source,java,diff-id=1,diff-type=noncompliant]
 ----
 import javax.persistence.Entity;
 
@@ -40,22 +68,16 @@ import org.springframework.web.bind.annotation.RequestMapping;
 @Controller
 public class WishListController {
 
-  @PostMapping(path = "/saveForLater")
-  public String saveForLater(Wish wish) {
-    session.save(wish);
-  }
-
   @RequestMapping(path = "/saveForLater", method = RequestMethod.POST)
-  public String saveForLater(Wish wish) {
+  public String saveForLater(Wish wish) { // Noncompliant
     session.save(wish);
   }
 }
 ----
 
+==== Compliant solution
 
-=== Compliant solution
-
-[source,java]
+[source,java,diff-id=1,diff-type=compliant]
 ----
 public class WishDTO {
   Long productId;
@@ -69,36 +91,40 @@ import org.springframework.web.bind.annotation.RequestMapping;
 @Controller
 public class PurchaseOrderController {
 
-  @PostMapping(path = "/saveForLater")
-  public String saveForLater(WishDTO wish) {
-    Wish persistentWish = new Wish();   
-    // do the mapping between "wish" and "persistentWish"
-    [...]
-    session.save(persistentWish);
-  }
-
   @RequestMapping(path = "/saveForLater", method = RequestMethod.POST)
   public String saveForLater(WishDTO wish) {
     Wish persistentWish = new Wish();   
-    // do the mapping between "wish" and "persistentWish"
-    [...]
+    persistentWish.productId = wish.productId
+    persistentWish.quantity = wish.quantity
+    persistentWish.client = getClientById(with.clientId)
     session.save(persistentWish);
   }
 }
 ----
 
+=== How does this work?
 
-=== Exceptions
+The compliant code implements a Data Transfer Object (DTO) layer. Instead of
+accepting a persistent `Wish` entity as a parameter, the vulnerable method
+accepts a `WishDTO` object with a safe, minimal set of properties. It then
+instantiates a persistent entity and initializes it based on the DTO properties'
+values. The resulting object can safely be persisted in the database.
 
-No issue is reported when the parameter is annotated with ``++@PathVariable++`` from Spring Framework, since the lookup will be done via id, the object cannot be forged on client side.
+== Resources
 
+=== Documentation
 
-== Resources
+* Jakarta EE Documentation - https://jakarta.ee/specifications/platform/10/apidocs/jakarta/servlet/http/httpservletrequest#getRequestedSessionId--[`HttpServletRequest` - `getRequestedSessionId`]
+
+=== Standards
+
+* OWASP - https://owasp.org/Top10/A08_2021-Software_and_Data_Integrity_Failures/[Top 10 2021 - Category A8 - Software and Data Integrity Failures]
+* OWASP - https://owasp.org/www-project-top-ten/2017/A5_2017-Broken_Access_Control[Top 10 2017 - Category A5 - Broken Access Control]
+* CWE - https://cwe.mitre.org/data/definitions/915[CWE-915 - Improperly Controlled Modification of Dynamically-Determined Object Attributes]
+
+=== Articles & blog posts
 
-* https://owasp.org/Top10/A08_2021-Software_and_Data_Integrity_Failures/[OWASP Top 10 2021 Category A8] - Software and Data Integrity Failures
-* https://owasp.org/www-project-top-ten/2017/A5_2017-Broken_Access_Control[OWASP Top 10 2017 Category A5] - Broken Access Control
-* https://cwe.mitre.org/data/definitions/915[MITRE, CWE-915] - Improperly Controlled Modification of Dynamically-Determined Object Attributes
-* https://o2platform.files.wordpress.com/2011/07/ounce_springframework_vulnerabilities.pdf[Two Security Vulnerabilities in the Spring Framework’s MVC by Ryan Berg and Dinis Cruz]
+OWASP O2 Platform Blog - https://o2platform.files.wordpress.com/2011/07/ounce_springframework_vulnerabilities.pdf[Two Security Vulnerabilities in the Spring Framework's MVC]
 
 ifdef::env-github,rspecator-view[]