Uploaded image for project: 'CXF'
  1. CXF
  2. CXF-7287

Incorrect JSON decoding in JsonMapObjectReaderWriter when commas are used in value

Attach filesAttach ScreenshotVotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 3.1.10
    • 3.1.11, 3.2.0
    • JAX-RS, JAX-RS Security
    • None
    • OSX, Tomcat8, Java8

    • Moderate

    Description

      Background:
      Discovered when using JWT tokens. In org.apache.cxf.rs.security.jose.jws.JwsJwtCompactConsumer there is a line of code that reads:

      JwtClaims theClaims = new JwtClaims(getReader().fromJson(getDecodedJwsPayload()));
      

      If the method "getDecodedJwsPayload()" returns a JSON string like:

      "\"sub\":\"admin\",\"roles\":\"admin,user\",\"iss\":\"auth0\""
      

      then the "fromJson" method returns an incorrect Map.

      The Bug
      The problem occurs in org.apache.cxf.jaxrs.json.basic.JsonMapObjectReaderWriter (defined in cxf-rt-rs-json-basic-3.1.10.jar) in the readJsonObjectAsSettable method. Specifically, line 188 reads:

      int commaIndex = getCommaIndex(json, sepIndex + j);
      

      which will cause the wrong comma index to be found for the JSON key "roles" in my example above (apparently commaIndex is used to find the "next json key" position in the json string).

      I'm also thinking that it's possible that line 166 could be a problem as well, if a JSON value would ever contain an (escaped) quote character.

      This JSON parsing seems fragile... I wonder why a "standard" JSON library wasn't used (perhaps just to not add an additional dependency?)

      Proper parsing of commas in JSON values is important for JWT purposes as the constructor of org.apache.cxf.rs.security.jose.jaxrs.JwtTokenSecurityContext expects role claims to be comma-separated.

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            sergey_beryozkin Sergey Beryozkin
            borice Boris Capitanu
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment