Struts 2
  1. Struts 2
  2. WW-3537

XSRF flaw in struts2/trunk/plugins/rest/src/main/java/org/apache/struts2/rest/RestActionMapper.java

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.2.1
    • Fix Version/s: 2.2.1.1
    • Component/s: Plugin - REST
    • Labels:
      None
    • Flags:
      Important

      Description

      I believe I've just found a major XSFR flaw in the REST plugin's RestActionMapper.

      See http://www.owasp.org/index.php/Cross-Site_Request_Forgery_%28CSRF%29 for more details concerning XSRF.

      Manually performing a GET request on a create() method using the name!method convention, the create() method actually gets invoked (btw, the model is also populated).
      As far as I can see, ANY of the operations with side effects (create, update, destroy) can be invoked this way (using a GET request)

      The code in RestActionMapper seems to totally ignore the HTTP-method used:

      // handle "name!method" convention.
      String name = mapping.getName();
      int exclamation = name.lastIndexOf("!");

      if (exclamation != -1)

      { mapping.setName(name.substring(0, exclamation)); mapping.setMethod(name.substring(exclamation + 1)); }

      Most other REST frameworks use annotations like @GET/@POST or similar mechanisms on the controller methods in order to make sure that the correct method is used, otherwise yielding a 400 BAD REQUEST or similar.

      Has this issue been addressed before?

      In the current state, I would not recommend using the REST plugin for production use.

        Activity

        Hide
        Stefan Magnus Landrø added a comment -

        Well, it's real hard to protect yourself against XSRF - however, enforcing the correct request method makes it a little bit harder to exploit and the attacker has to leave more traces.

        Show
        Stefan Magnus Landrø added a comment - Well, it's real hard to protect yourself against XSRF - however, enforcing the correct request method makes it a little bit harder to exploit and the attacker has to leave more traces.
        Hide
        Dave Newton added a comment -

        Oh, I see--I didn't know that wasn't inherited I guess. Good catch, then.

        Although I still don't get the XSRF part.

        Show
        Dave Newton added a comment - Oh, I see--I didn't know that wasn't inherited I guess. Good catch, then. Although I still don't get the XSRF part.
        Hide
        Dave Newton added a comment -

        Isn't this just the old issue that culminated in dynamic method invocation being false by default?

        (I guess I'm not sure how this is XSRF/request-type related other than you could validate the type, but I don't see how that prevents XSRF--isn't that explicitly stated on the reference you cite?)

        Show
        Dave Newton added a comment - Isn't this just the old issue that culminated in dynamic method invocation being false by default? (I guess I'm not sure how this is XSRF/request-type related other than you could validate the type, but I don't see how that prevents XSRF--isn't that explicitly stated on the reference you cite?)
        Hide
        Lukasz Lenart added a comment -

        Implemented DMI support in RestActionMapper, so it can be disabled by specifying

        struts.enable.DynamicMethodInvocation = false

        Probably in the next major release, DMI support will be removed and fully disabled!

        Thanks for reporting!

        Show
        Lukasz Lenart added a comment - Implemented DMI support in RestActionMapper, so it can be disabled by specifying struts.enable.DynamicMethodInvocation = false Probably in the next major release, DMI support will be removed and fully disabled! Thanks for reporting!

          People

          • Assignee:
            Lukasz Lenart
            Reporter:
            Stefan Magnus Landrø
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development