S2755 rule triggers for local variables but not fields

Clean Code Report False-positive / False-negative...
1

Hi,

The S2755 rule on XXE attacks triggers if the XML parser is a local variable but not if it is a field.

  • Operating system: Windows
  • SonarLint plugin version: 7.4
  • Is connected mode used: No

Here’s a minimal reproducible example with both situations when XML parser is local variable and when it is a field

import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
import org.xml.sax.XMLReader;
import org.xml.sax.helpers.DefaultHandler;
import org.xml.sax.helpers.XMLReaderFactory;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;

import javax.xml.parsers.ParserConfigurationException;
import javax.xml.parsers.SAXParser;
import javax.xml.parsers.SAXParserFactory;

public class TestXXE {

    private SAXParserFactory saxParserFactory = SAXParserFactory.newInstance();
    private SAXParser saxParser;

    private XMLReader reader;
    
    public TestXXE() throws ParserConfigurationException, SAXException {
        saxParser = saxParserFactory.newSAXParser();
    }

    public void testOK(File file, DefaultHandler handler) throws SAXException, IOException, ParserConfigurationException {
        SAXParserFactory localSaxParserFactory = SAXParserFactory.newInstance();
        localSaxParserFactory.setFeature("http://xml.org/sax/features/external-general-entities", false);
        localSaxParserFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
        SAXParser localSAXParser = localSaxParserFactory.newSAXParser();
        localSAXParser.parse(file, handler);
        
        @SuppressWarnings("deprecation")
        XMLReader localReader = XMLReaderFactory.createXMLReader();
        try (FileInputStream fileInputStream = new FileInputStream(file)) {
            localReader.parse(new InputSource(fileInputStream));
        }
    }

    @SuppressWarnings("deprecation")
    public void testKO(File file, DefaultHandler handler) throws SAXException, IOException {
        saxParser.parse(file, handler);
        reader = XMLReaderFactory.createXMLReader(); // XXE warning expected here
        try (FileInputStream fileInputStream = new FileInputStream(file)) {
            reader.parse(new InputSource(fileInputStream));
        }
    }
}

Thanks & regards,
Nicolas.

2

Hello Nicolas,

Thanks for raising this. I am able to reproduce the problem with your code snippet. I will notify the team in charge.

Have a good week-end :slight_smile:

1 Like
5

Hello @Nicolas_Baumann and thank you for your patience.

What you observe here is a known limitation of the engine on which the rule S2755 is based. Our open-source java symbolic execution engine is not field-sensitive. (Non-final) fields can be potentially modified from anywhere in the code, making it extremely hard to detect without generating too much noise and tons of False Positives in counterpart. We don’t have plans to mitigate this limitation anytime soon, and consequently won’t be able to detect such cases.

Cheers,
Michael