Uploaded image for project: 'SonarJava'
  1. SonarJava
  2. SONARJAVA-3780

s2755 should not raise when xml object is returned

    Details

    • Type: False-Positive
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: Rules
    • Labels:
      None

      Description

      A SonarCloud user provided this feedback,

      public static DocumentBuilderFactory getNsAwareDocumentBuilderFactory() throws ParserConfigurationException {
          DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
          setFeature(dbf, "http://apache.org/xml/features/disallow-doctype-decl"); // Compliant when set to true
      
         return dbf;
      }
      
      public String s2755compliantDocumentBuilderFactory() throws IOException {
          DocumentBuilder builder = getNsAwareDocumentBuilderFactory().newDocumentBuilder(); // FP
          Document document = builder.parse(is);
      }
      

      the feature ("http://apache.org/xml/features/disallow-doctype-decl") that turns the DocumentBuilderFactory into a safe factory is set in another function (setFeature) and thus we will loose track of it and we raise a FP.

      To avoid that, for all the xml objects (DocumentBuilderFactory, XMLInputfactory ...) we should raise if we see, the creation of the instance and the "parse" method in the same function AND between them the xml object is:

      • not returned
        public static DocumentBuilderFactory returned() throws ParserConfigurationException {
           DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); // Compliant
           return dbf; // returned so we consider it safe because can be secured in the caller
        }
        
        public static DocumentBuilderFactory returned() throws ParserConfigurationException {
           InputStream is = classLoader.getResourceAsStream(xml);
           DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); // Noncompliant
           DocumentBuilder builder = dbf.newDocumentBuilder();
           builder.parse(is);
           return dbf; // returned but the parse is before
        }
        
      • not passed as an argument of another function
        public static void argument() throws ParserConfigurationException {
            DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); // Compliant
            otherfunction(dbf); // passed as argument of another function
        }
        
        public static void argument() throws ParserConfigurationException {
           InputStream is = classLoader.getResourceAsStream(xml);
           DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance(); // Noncompliant
           DocumentBuilder builder = dbf.newDocumentBuilder();
           builder.parse(is);
          otherfunction(dbf); // passed as argument of another function but the parse is before
        }
        

      Code examples: https://github.com/SonarSource/security-expected-issues/pull/390

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                Unassigned
                Reporter:
                eric.therond Eric Therond
              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated: