struts-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Siman (JIRA)" <j...@apache.org>
Subject [jira] Updated: (WW-3264) Vulnerability of dynamic method invocation
Date Sat, 26 Sep 2009 01:52:48 GMT

     [ https://issues.apache.org/struts/browse/WW-3264?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Alex Siman updated WW-3264:
---------------------------

    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))
        {
            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";
    }
}



  was:
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";
	}
}



> Vulnerability of dynamic method invocation
> ------------------------------------------
>
>                 Key: WW-3264
>                 URL: https://issues.apache.org/struts/browse/WW-3264
>             Project: Struts 2
>          Issue Type: Bug
>    Affects Versions: 2.1.8
>            Reporter: Alex Siman
>            Priority: Critical
>
> 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.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message