Details
-
Bug
-
Status: Closed
-
Critical
-
Resolution: Fixed
-
2.1.8
-
None
-
None
Description
Dynamic method invocation is the security hole. If some of action methods has "public" visibility and return String, then attacker can call this method. In the example below, attacker can call method "changeAdminPassword()" of TestAction from URL like:
http://example.com/test!changeAdminPassword.action
public class TestAction
{
private String currentPassword = null;
@SkipValidation
public String execute() throws Exception
{
if (getValidCurrentPassword().equals(currentPassword))
else
{ addFieldError("currentPassword", "Invalid password."); return INPUT; }}
// Note "public" visibility here.
public String changeAdminPassword()
public String getCurrentPassword()
{ return currentPassword; }public void setCurrentPassword(String currentPassword)
{ this.currentPassword = currentPassword; }}
To fix this vulnerability we must leverage the [com.opensymphony.xwork2.config.entities.ActionConfig.allowedMethods].
And to prevent backward incompatibility add new default setting like:
default.properties
==================
-
- Note "false".
struts.enable.DynamicMethodInvocation.restrictToAllowedMethods = false
- Note "false".
Desired code in struts.xml
==========================
<package name="testPackage">
<action name="login" class="actions.LoginAction">
<result>/login.jsp</result>
<allowedMethods>
<allowedMethod>doLogin</allowedMethod>
</allowedMethods>
</action>
<allowedMethods class="actions.LoginAction">
<allowedMethod>doLogin</allowedMethod>
<allowedMethod>doRegister</allowedMethod>
</allowedMethods>
<!— Or use <method>? -->
<allowedMethods class="actions.UserAction">
<method>create</method>
<method>list</method>
<method>view</method>
<!-- ... -->
</allowedMethods>
</package>
Desired code w/ Convention plugin:
(Note @AllowedMethod anno)
==================================
class LoginAction
{
@SkipValidation
public String execute()
@AllowedMethod
public String doLogin()
// This method cannot be invoked dynamically.
public String getPasswordHash()
}