Spring Security
  1. Spring Security
  2. SEC-1753

Replay of response causes NullPointerException in OpenID4JavaConsumer

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: None
    • Fix Version/s: 3.1.0.RC3
    • Component/s: OpenID
    • Labels:
      None

      Description

      discovered may be null which causes NullPointerException

      // retrieve the previously stored discovery information
      DiscoveryInformation discovered = (DiscoveryInformation) request.getSession().getAttribute(DISCOVERY_INFO_KEY);
      List<OpenIDAttribute> attributesToFetch = (List<OpenIDAttribute>) request.getSession().getAttribute(ATTRIBUTE_LIST_KEY);

      request.getSession().removeAttribute(DISCOVERY_INFO_KEY);
      request.getSession().removeAttribute(ATTRIBUTE_LIST_KEY);
      ..
      Identifier id = discovered.getClaimedIdentifier();

        Activity

        Hide
        Luke Taylor added a comment -

        Could you clarify the steps which lead to this behaviour please.

        Show
        Luke Taylor added a comment - Could you clarify the steps which lead to this behaviour please.
        Hide
        Simo Nikula added a comment -

        This case happens during hacking/cracking attempt (thats why it is minor).
        I was comparing openid packages and checked how they handle case where response from OpenID Provider is replayed.
        Security is ok in above implementation as data is removed from session after it has been used but diagnostics from NullPointerException is not too good.

        You may have better idea but something like
        if(null==discovered)

        { throw new OpenIDConsumerException("DiscoveryInformation is not available, Possible causes are e.g. lost session or replay attack"); }

        Other option that I prefer would be not to remove DiscoveryInformation from session but let ConsumerManager._nonceVerifier.seen() report possible attack

        Show
        Simo Nikula added a comment - This case happens during hacking/cracking attempt (thats why it is minor). I was comparing openid packages and checked how they handle case where response from OpenID Provider is replayed. Security is ok in above implementation as data is removed from session after it has been used but diagnostics from NullPointerException is not too good. You may have better idea but something like if(null==discovered) { throw new OpenIDConsumerException("DiscoveryInformation is not available, Possible causes are e.g. lost session or replay attack"); } Other option that I prefer would be not to remove DiscoveryInformation from session but let ConsumerManager._nonceVerifier.seen() report possible attack
        Hide
        Luke Taylor added a comment -

        OK, I've added a check for the missing DiscoveryInformation as you suggest. If you want to retain the DiscoveryInformation in the session for the duration you can override the endConsumption method and put it back after calling super.

        Show
        Luke Taylor added a comment - OK, I've added a check for the missing DiscoveryInformation as you suggest. If you want to retain the DiscoveryInformation in the session for the duration you can override the endConsumption method and put it back after calling super.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: