16f6c0aecf
Inline adoc files when they are included exactly once. Also fix language tags because this inlining gives us better information on what language the code is written in.
92 lines
2.7 KiB
Plaintext
92 lines
2.7 KiB
Plaintext
Using setters in Struts 2 ActionSupport is security-sensitive. For example, their use has led in the past to the following vulnerabilities:
|
|
|
|
* http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2012-1006[CVE-2012-1006]
|
|
|
|
All classes extending ``++com.opensymphony.xwork2.ActionSupport++`` are potentially remotely reachable. An action class extending ActionSupport will receive all HTTP parameters sent and these parameters will be automatically mapped to the setters of the Struts 2 action class. One should review the use of the fields set by the setters, to be sure they are used safely. By default, they should be considered as untrusted inputs.
|
|
|
|
|
|
== Ask Yourself Whether
|
|
|
|
* the setter is needed. There is no need for it if the attribute's goal is not to map queries' parameter.
|
|
* the value provided to the setter is properly sanitized before being used or stored. (*)
|
|
|
|
(*) You are at risk if you answered yes to this question.
|
|
|
|
|
|
== Recommended Secure Coding Practices
|
|
|
|
As said in Struts documentation: https://struts.apache.org/security/#do-not-define-setters-when-not-needed["Do not define setters when not needed"]
|
|
|
|
Sanitize the user input. This can be for example done by implementing the ``++validate()++`` method of ``++com.opensymphony.xwork2.ActionSupport++``.
|
|
|
|
|
|
== Sensitive Code Example
|
|
|
|
[source,java]
|
|
----
|
|
public class AccountBalanceAction extends ActionSupport {
|
|
private static final long serialVersionUID = 1L;
|
|
private Integer accountId;
|
|
|
|
// this setter might be called with user input
|
|
public void setAccountId(Integer accountId) {
|
|
this.accountId = accountId;
|
|
}
|
|
|
|
@Override
|
|
public String execute() throws Exception {
|
|
// call a service to get the account's details and its balance
|
|
[...]
|
|
return SUCCESS;
|
|
}
|
|
}
|
|
----
|
|
|
|
|
|
== See
|
|
|
|
* https://owasp.org/www-project-top-ten/2017/A1_2017-Injection[OWASP Top 10 2017 Category A1] - Injection
|
|
|
|
|
|
ifdef::env-github,rspecator-view[]
|
|
|
|
'''
|
|
== Implementation Specification
|
|
(visible only on this page)
|
|
|
|
=== Message
|
|
|
|
Make sure that executing this ActionSupport is safe.
|
|
|
|
|
|
=== Highlighting
|
|
|
|
First: the ``++execute++`` method
|
|
|
|
Second: locations where the setters are defined.
|
|
|
|
|
|
'''
|
|
== Comments And Links
|
|
(visible only on this page)
|
|
|
|
=== is related to: S4529
|
|
|
|
=== on 26 Mar 2018, 20:52:10 Alexandre Gigleux wrote:
|
|
Struts 2 Examples: \https://github.com/apache/struts-examples
|
|
|
|
|
|
This rule should raise an issue if:
|
|
|
|
* the class is extending ``++com.opensymphony.xwork2.ActionSupport++``
|
|
* the class overrides the ``++execute++`` method
|
|
* the class is having at least one setter method
|
|
|
|
=== on 26 Mar 2018, 20:56:59 Alexandre Gigleux wrote:
|
|
This is a "Security Hotspot".
|
|
|
|
=== on 27 May 2020, 16:47:21 Eric Therond wrote:
|
|
Deprecated because it overlaps with SonarSecurity
|
|
|
|
endif::env-github,rspecator-view[]
|