[
https://issues.apache.org/jira/browse/WW-3264?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13031456#comment-13031456
]
Diego Díez commented on WW-3264:
--------------------------------
John, Could you post your own implementation to fix this issue?
Which files I have to modify? ActionConfig.java, the DTD, Anything else?
Thanks in advantage.
> Vulnerability of dynamic method invocation
> ------------------------------------------
>
> Key: WW-3264
> URL: https://issues.apache.org/jira/browse/WW-3264
> Project: Struts 2
> Issue Type: Bug
> Affects Versions: 2.1.8
> Reporter: Alex Siman
> Assignee: John Lindal
> Priority: Critical
> Fix For: 2.2.x
>
>
> 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))
> {
> String feedback = changeAdminPassword();
> addActionMessage(feedback);
> return SUCCESS;
> }
> else
> {
> addFieldError("currentPassword", "Invalid password.");
> return INPUT;
> }
> }
> // Note "public" visibility here.
> public String changeAdminPassword()
> {
> String newPassword = "new-admin";
> // Persist changes here...
> return "Admin password has been changed to '" + newPassword + "'.";
> }
>
> 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
> 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()
> {
> // Nothing.
> return INPUT;
> }
> @AllowedMethod
> public String doLogin()
> {
> // Method's body here...
> // password = getPasswordHash();
> return SUCCESS;
> }
> // This method cannot be invoked dynamically.
> public String getPasswordHash()
> {
> // Method's body here...
> return "xxx";
> }
> }
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
