rspec/rules/S4684/java/rule.adoc

== 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. 

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.


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.


For this reason, using ``++@Entity++`` or ``++@Document++`` objects as arguments of methods annotated with ``++@RequestMapping++`` should be avoided.


In addition to ``++@RequestMapping++``, this rule also considers the annotations introduced in Spring Framework 4.3: ``++@GetMapping++``, ``++@PostMapping++``, ``++@PutMapping++``, ``++@DeleteMapping++``, ``++@PatchMapping++``.


=== Noncompliant code example

[source,java]
----
import javax.persistence.Entity;

@Entity
public class Wish {
  Long productId;
  Long quantity;
  Client client;
}

@Entity
public class Client {
  String clientId;
  String name;
  String password;
}

import org.springframework.stereotype.Controller;
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) {
    session.save(wish);
  }
}
----


=== Compliant solution

[source,java]
----
public class WishDTO {
  Long productId;
  Long quantity;
  Long clientId;
}

import org.springframework.stereotype.Controller;
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"
    [...]
    session.save(persistentWish);
  }
}
----


=== Exceptions

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

* 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]

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

Replace this persistent entity with a simple POJO or DTO object.


endif::env-github,rspecator-view[]