Struts 2
  1. Struts 2
  2. WW-3631

Implementing SessionAware allows session tampering

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1.8.1
    • Fix Version/s: 2.3.3
    • Component/s: Value Stack
    • Labels:
    • Environment:

      Tested using Glassfish v3.

      Description

      This was previously raised as an issue under WW-2264. After the discussion it was determined that this is not a bug - I disagree and would like to raise the issue again.

      If an Action implements SessionAware the contents of the session are modifiable, this includes the public setters on objects stored in the session.

      Ok, for the Action to be able to modify the contents of the session it must also implement a "public Map getSession()". However, even if the Action does not implement a getSession method it is still possible for an attacker to tamper with the contents of the HttpSession and affect the processesing of the Action.

      I agree with the solutions previously discussed in WW-2264 that 'session' should be added to the parameter exclusion list in the struts-default.xml. Additionally, a warning should be added to the JavaDoc for SessionAware indicating the possible issue with exposing the session via the interface and that if the configuration of the intercepters does not explicitly exclude 'session' in the paramExclude node that it is possible for a requester to modify the session.

      1. Struts2Test.zip
        3.10 MB
        Jeremy Long

        Activity

        Hide
        Hudson added a comment -

        Integrated in Struts2 #427 (See https://builds.apache.org/job/Struts2/427/)
        WW-3631 - adds additional excluded params to avoid session / request / response tempering (Revision 1297521)

        Result = SUCCESS
        lukaszlenart :
        Files :

        • /struts/struts2/trunk/core/src/main/resources/struts-default.xml
        Show
        Hudson added a comment - Integrated in Struts2 #427 (See https://builds.apache.org/job/Struts2/427/ ) WW-3631 - adds additional excluded params to avoid session / request / response tempering (Revision 1297521) Result = SUCCESS lukaszlenart : Files : /struts/struts2/trunk/core/src/main/resources/struts-default.xml
        Hide
        Lukasz Lenart added a comment -

        New excludedParams committed, thanks for help!

        Show
        Lukasz Lenart added a comment - New excludedParams committed, thanks for help!
        Hide
        Jeremy Long added a comment -

        Lukasz,

        I believe Abraham Kang found an different issues with ServletRequestAware
        but I don't know if he has notified anyone of it yet. I cced him as it
        would be trivial to add a block for that too while fixing this issue.

        With regards to the other *Aware interfaces I couldn't find a way to
        exploit ServletResponseAware or ParameterAware - not saying there aren't
        other issues there, but I couldn't come up with anything. However, I might
        suggest proactively protecting the ServletResponseAware and ParameterAware
        too.

        Abe - anything else?

        So the excludeParams should be:

        <param
        name="excludeParams">dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,parameters\...*</param>
        

        --Jeremy

        On Fri, Feb 24, 2012 at 3:34 AM, Lukasz Lenart (Commented) (JIRA) <

        Show
        Jeremy Long added a comment - Lukasz, I believe Abraham Kang found an different issues with ServletRequestAware but I don't know if he has notified anyone of it yet. I cced him as it would be trivial to add a block for that too while fixing this issue. With regards to the other *Aware interfaces I couldn't find a way to exploit ServletResponseAware or ParameterAware - not saying there aren't other issues there, but I couldn't come up with anything. However, I might suggest proactively protecting the ServletResponseAware and ParameterAware too. Abe - anything else? So the excludeParams should be: <param name= "excludeParams" >dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*,^servlet(Request|Response)\..*,parameters\...* </param> --Jeremy On Fri, Feb 24, 2012 at 3:34 AM, Lukasz Lenart (Commented) (JIRA) <
        Hide
        Lukasz Lenart added a comment -
        <param name="excludeParams">dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..*</param>
        
        Show
        Lukasz Lenart added a comment - <param name= "excludeParams" > dojo\..*,^struts\..*,^session\..*,^request\..*,^application\..* </param>
        Hide
        Lukasz Lenart added a comment -

        So adding additional ignore params (/^session\../, /^request\../( should solve the problem ?

        Show
        Lukasz Lenart added a comment - So adding additional ignore params (/^session\.. /, /^request\.. /( should solve the problem ?
        Hide
        Lukasz Lenart added a comment -
        Show
        Lukasz Lenart added a comment - That blog post should also be linked here http://codesecure.blogspot.com/2011/12/struts-2-session-tampering-via.html
        Hide
        Lukasz Lenart added a comment -

        Looks more important than I thought

        Show
        Lukasz Lenart added a comment - Looks more important than I thought
        Hide
        Jeremy Long added a comment -

        Thanks - so what about even Struts 2 use of this interface in things such
        as DirectRenderFromEventAction:
        http://struts.apache.org/2.0.11/struts2-core/apidocs/org/apache/struts2/portlet/dispatcher/DirectRenderFromEventAction.html

        I have not used or seen the Struts2 Portlets in use, so I'm unsure whether
        a problem might exist there.

        Lastly - while not putting an actual fix in place, could the JavaDoc at
        least be updated to indicate that there is a possible security concern with
        implementing SessionAware or RequestAware?

        Thanks,

        --Jeremy

        On Sun, Dec 4, 2011 at 10:39 AM, Dave Newton (Commented) (JIRA) <

        Show
        Jeremy Long added a comment - Thanks - so what about even Struts 2 use of this interface in things such as DirectRenderFromEventAction: http://struts.apache.org/2.0.11/struts2-core/apidocs/org/apache/struts2/portlet/dispatcher/DirectRenderFromEventAction.html I have not used or seen the Struts2 Portlets in use, so I'm unsure whether a problem might exist there. Lastly - while not putting an actual fix in place, could the JavaDoc at least be updated to indicate that there is a possible security concern with implementing SessionAware or RequestAware? Thanks, --Jeremy On Sun, Dec 4, 2011 at 10:39 AM, Dave Newton (Commented) (JIRA) <
        Hide
        Dave Newton added a comment -

        There is no release timeline.

        The solution then, apparently not implemented, was to add session to the list of ignored params (approximately /^session\..*/) which seems reasonable; it can be modified if someone really wants to allow it. Same with anything else like that, e.g., request.

        Pretty easy work-around in existing apps by configuring the interceptor at the package/action level, or in the action itself via ParameterNameAware.

        Certainly no need to wait for anything.

        Show
        Dave Newton added a comment - There is no release timeline. The solution then, apparently not implemented, was to add session to the list of ignored params (approximately /^session\..*/ ) which seems reasonable; it can be modified if someone really wants to allow it. Same with anything else like that, e.g., request. Pretty easy work-around in existing apps by configuring the interceptor at the package/action level, or in the action itself via ParameterNameAware . Certainly no need to wait for anything.
        Hide
        Jeremy Long added a comment -

        Lukasz,

        Could you tell me what the fix will be in 3.x and what kind of timeline
        there is for the 3.x version?

        --Jeremy

        On Sun, Dec 4, 2011 at 6:03 AM, Lukasz Lenart (Updated) (JIRA) <

        Show
        Jeremy Long added a comment - Lukasz, Could you tell me what the fix will be in 3.x and what kind of timeline there is for the 3.x version? --Jeremy On Sun, Dec 4, 2011 at 6:03 AM, Lukasz Lenart (Updated) (JIRA) <
        Hide
        Jeremy Long added a comment -

        This is also an issue for RequestAware. If an Action implements RequestAware and creates the associated getRequest method - then one can tamper not only with the request attributes, but also the session attributes and public setters on objects stored in the session.

        Show
        Jeremy Long added a comment - This is also an issue for RequestAware. If an Action implements RequestAware and creates the associated getRequest method - then one can tamper not only with the request attributes, but also the session attributes and public setters on objects stored in the session.
        Hide
        Jeremy Long added a comment -

        The attached code is a Netbeans project that demos the issue discussed.

        Show
        Jeremy Long added a comment - The attached code is a Netbeans project that demos the issue discussed.

          People

          • Assignee:
            Lukasz Lenart
            Reporter:
            Jeremy Long
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development