Spring Security
  1. Spring Security
  2. SEC-812

Framework can help protect against XSS attacks on login page by escaping stored last username

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0.2
    • Component/s: Core
    • Labels:
      None
    • Environment:
      Windows Server 2003

      Description

      During a penetration test the username field in j_acegi_security_check was found to be vulnerable to XSS. Additionally the XSS seems to preform a pseudo SQL attack with the following instructions, note this can only be reproduced with FireFox.

      Open FireFox and visit http:/host/j_acegi_security_check?j_username=%27%3Cscript%3Ealert%28%27xss%27%29%3C%2Fscript%3E&j_password=d&login=Login

      A session error should occur.

      Kill or end the FireFox process.

      Reopen FireFox with the "Restore Session" option.

      User data should now be dumped in the browser.

        Activity

        Hide
        Luke Taylor added a comment -

        Thanks for the report. We'll look into it. Could you possibly expand a bit on what kind of setup you were running against? I can see how the XSS can occur without validation of the entered username, but some more info would be useful.

        Show
        Luke Taylor added a comment - Thanks for the report. We'll look into it. Could you possibly expand a bit on what kind of setup you were running against? I can see how the XSS can occur without validation of the entered username, but some more info would be useful.
        Hide
        Luke Taylor added a comment -

        The username parameter issue isn't a problem unless someone re-renders the value into a web page without escaping (which they will get automatically if using JSTL's <c:out /> tag, for example). So it could be argued that it's not the framework's responsibility to sanitize the submitted value, but the application developer's. The client may not even be a browser. However since most applications are unlikely to have a requirement to allow HTML characters in usernames we could consider sanitizing the value to prevent this kind of thing completely. Again, it would be useful to know how the login page was being rendered in the application you were testing.

        We should also check that the sample applications don't suffer from this to illustrate best practice and raise it as a potential issue in the docs somewhere.

        Show
        Luke Taylor added a comment - The username parameter issue isn't a problem unless someone re-renders the value into a web page without escaping (which they will get automatically if using JSTL's <c:out /> tag, for example). So it could be argued that it's not the framework's responsibility to sanitize the submitted value, but the application developer's. The client may not even be a browser. However since most applications are unlikely to have a requirement to allow HTML characters in usernames we could consider sanitizing the value to prevent this kind of thing completely. Again, it would be useful to know how the login page was being rendered in the application you were testing. We should also check that the sample applications don't suffer from this to illustrate best practice and raise it as a potential issue in the docs somewhere.
        Hide
        Chris Castaldo added a comment -

        I am collecting more information from the user. I will have your questions answered hopefully by Wednesday.

        Show
        Chris Castaldo added a comment - I am collecting more information from the user. I will have your questions answered hopefully by Wednesday.
        Hide
        Chris Castaldo added a comment -

        Spring Version - 1.2.6

        Acegi Version - 0.9.0

        The Acegi Version has been extended to incorporate DB based
        authentication.

        Let me know if I can provide anymore details.

        Show
        Chris Castaldo added a comment - Spring Version - 1.2.6 Acegi Version - 0.9.0 The Acegi Version has been extended to incorporate DB based authentication. Let me know if I can provide anymore details.
        Hide
        Luke Taylor added a comment -

        Thanks. I guess the question is really how they are rendering their login page - they must be resinserting the username into the page after a failed login and it's at that point that they should escape the value to replace the <>"' character with HTML entities.

        Show
        Luke Taylor added a comment - Thanks. I guess the question is really how they are rendering their login page - they must be resinserting the username into the page after a failed login and it's at that point that they should escape the value to replace the <>"' character with HTML entities.
        Hide
        Luke Taylor added a comment -

        Do you have any more information on how they are rendering the page?

        I have updated the code to escape the username value which is stored in the session, so in future this should prevent users from inadvertently running into this problem. I don't think it's a bug in the framework though.

        Show
        Luke Taylor added a comment - Do you have any more information on how they are rendering the page? I have updated the code to escape the username value which is stored in the session, so in future this should prevent users from inadvertently running into this problem. I don't think it's a bug in the framework though.
        Hide
        Luke Taylor added a comment - - edited

        Changed title to reflect that this isn't a bug in Spring Security but rather an issue with rendering the application's login page with a previously entered username.

        Show
        Luke Taylor added a comment - - edited Changed title to reflect that this isn't a bug in Spring Security but rather an issue with rendering the application's login page with a previously entered username.
        Hide
        Luke Taylor added a comment -

        The previous username is now stored with XML escaping enabled.

        Show
        Luke Taylor added a comment - The previous username is now stored with XML escaping enabled.

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Chris Castaldo
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: