diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/dast/StreamingWebInspectParser.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/dast/StreamingWebInspectParser.java index f6ac5ec2cf3..01dee42b73f 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/dast/StreamingWebInspectParser.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/dast/StreamingWebInspectParser.java @@ -34,6 +34,7 @@ import com.fortify.cli.aviator._common.exception.AviatorTechnicalException; import com.fortify.cli.aviator.util.FprHandle; +import com.fortify.cli.common.util.SecureXmlParserFactory; /** * Streaming (StAX-based) parser for WebInspect XML files contained in DAST FPR files. @@ -48,9 +49,7 @@ public class StreamingWebInspectParser { public StreamingWebInspectParser(FprHandle fprHandle) { this.fprHandle = fprHandle; - this.xmlInputFactory = XMLInputFactory.newInstance(); - xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); - xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + this.xmlInputFactory = SecureXmlParserFactory.newXmlInputFactory(); } /** diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/dast/WebInspectParser.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/dast/WebInspectParser.java index d41a998a813..f3627a7e598 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/dast/WebInspectParser.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/dast/WebInspectParser.java @@ -21,9 +21,7 @@ import java.util.Base64; import java.util.List; -import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import org.slf4j.Logger; @@ -34,6 +32,7 @@ import org.xml.sax.SAXException; import com.fortify.cli.aviator.util.FprHandle; +import com.fortify.cli.common.util.SecureXmlParserFactory; /** * Parser for WebInspect XML files contained in DAST FPR files. @@ -62,7 +61,7 @@ public List parse() { } try (InputStream inputStream = Files.newInputStream(webInspectPath)) { - DocumentBuilderFactory factory = createSecureDocumentBuilderFactory(); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(false); DocumentBuilder builder = factory.newDocumentBuilder(); Document document = builder.parse(inputStream); @@ -114,7 +113,7 @@ public List parseSessions() { } try (InputStream inputStream = Files.newInputStream(webInspectPath)) { - DocumentBuilderFactory factory = createSecureDocumentBuilderFactory(); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(false); DocumentBuilder builder = factory.newDocumentBuilder(); Document document = builder.parse(inputStream); @@ -432,14 +431,4 @@ private String stripHtmlTags(String html) { .trim(); } - private DocumentBuilderFactory createSecureDocumentBuilderFactory() throws ParserConfigurationException { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setXIncludeAware(false); - factory.setExpandEntityReferences(false); - return factory; - } } diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/model/FPRInfo.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/model/FPRInfo.java index 3e5816a67be..6c17c387b51 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/model/FPRInfo.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/model/FPRInfo.java @@ -19,7 +19,6 @@ import java.util.Optional; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.stream.XMLInputFactory; import javax.xml.stream.XMLStreamConstants; import javax.xml.stream.XMLStreamException; @@ -36,6 +35,7 @@ import com.fortify.cli.aviator.fpr.filter.FilterTemplate; import com.fortify.cli.aviator.util.FprHandle; import com.fortify.cli.aviator.util.StringUtil; +import com.fortify.cli.common.util.SecureXmlParserFactory; import lombok.Getter; import lombok.Setter; @@ -80,10 +80,7 @@ private void extractInfoFromAuditFvdlStreaming(FprHandle fprHandle) { throw new AviatorTechnicalException("audit.fvdl not found in FPR: " + fprHandle.getFprPath()); } - XMLInputFactory factory = XMLInputFactory.newInstance(); - // Security: Disable external entity processing - factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); - factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + XMLInputFactory factory = SecureXmlParserFactory.newXmlInputFactory(); try (InputStream inputStream = Files.newInputStream(auditPath)) { XMLStreamReader reader = factory.createXMLStreamReader(inputStream); @@ -187,13 +184,7 @@ private void extractInfoFromAuditFvdl(FprHandle fprHandle) throws Exception { throw new AviatorTechnicalException("audit.fvdl not found in FPR: " + fprHandle.getFprPath()); } - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - factory.setXIncludeAware(false); - factory.setExpandEntityReferences(false); - factory.setValidating(false); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(false); DocumentBuilder builder = factory.newDocumentBuilder(); Document auditDoc; diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/model/Node.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/model/Node.java index c9cb6cdc660..9c2c07c8823 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/model/Node.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/model/Node.java @@ -21,9 +21,7 @@ import java.util.Optional; import java.util.regex.Pattern; -import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -33,6 +31,7 @@ import com.fortify.cli.aviator.fpr.filter.FilterTemplate; import com.fortify.cli.aviator.fpr.utils.Searchable; import com.fortify.cli.aviator.util.StringUtil; +import com.fortify.cli.common.util.SecureXmlParserFactory; import lombok.Getter; import lombok.Setter; @@ -320,13 +319,7 @@ private void extractInfoFromAuditFvdl(Path extractedPath) throws Exception { throw new IllegalStateException("audit.fvdl not found in " + extractedPath); } - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - factory.setFeature("http://xml.org/sax/features/validation", false); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(false); DocumentBuilder builder = factory.newDocumentBuilder(); Document auditDoc = builder.parse(auditPath.toFile()); @@ -365,10 +358,7 @@ private void extractInfoFromAuditFvdlStreaming(Path extractedPath) throws Except throw new IllegalStateException("audit.fvdl not found in " + extractedPath); } - javax.xml.stream.XMLInputFactory factory = javax.xml.stream.XMLInputFactory.newInstance(); - // Security: Disable external entity processing - factory.setProperty(javax.xml.stream.XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); - factory.setProperty(javax.xml.stream.XMLInputFactory.SUPPORT_DTD, false); + var factory = SecureXmlParserFactory.newXmlInputFactory(); try (java.io.InputStream inputStream = Files.newInputStream(auditPath)) { javax.xml.stream.XMLStreamReader reader = factory.createXMLStreamReader(inputStream); diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/AuditProcessor.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/AuditProcessor.java index 14e9a68dbc4..5183d371834 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/AuditProcessor.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/AuditProcessor.java @@ -37,7 +37,6 @@ import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; @@ -69,6 +68,7 @@ import com.fortify.cli.aviator.util.Constants; import com.fortify.cli.aviator.util.FileUtil; import com.fortify.cli.aviator.util.FprHandle; +import com.fortify.cli.common.util.SecureXmlParserFactory; import lombok.Setter; @@ -149,14 +149,7 @@ public Map processAuditXML() throws AviatorTechnicalExceptio logger.debug("audit.xml not found. Creating a default audit.xml."); auditDoc = createDefaultAuditXml(); } else { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - factory.setXIncludeAware(false); - factory.setExpandEntityReferences(false); - factory.setNamespaceAware(true); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(true); DocumentBuilder builder = factory.newDocumentBuilder(); try (InputStream auditStream = Files.newInputStream(auditPath)) { @@ -184,14 +177,7 @@ public Map processAuditXML() throws AviatorTechnicalExceptio private Document createDefaultAuditXml() throws AviatorTechnicalException { try { - DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); - docFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - docFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); - docFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - docFactory.setXIncludeAware(false); - docFactory.setExpandEntityReferences(false); - docFactory.setNamespaceAware(true); + var docFactory = SecureXmlParserFactory.newDocumentBuilderFactory(true); DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); Document doc = docBuilder.newDocument(); @@ -925,14 +911,7 @@ private Document generateRemediationsXml(Map auditRespons FPRInfo fprInfo, FVDLMetadata fvdlMetadata, Map skippedByReason) throws AviatorTechnicalException { try { - DocumentBuilderFactory docFactory = DocumentBuilderFactory.newInstance(); - docFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - docFactory.setFeature("http://xml.org/sax/features/external-general-entities", false); - docFactory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - docFactory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - docFactory.setXIncludeAware(false); - docFactory.setExpandEntityReferences(false); - docFactory.setNamespaceAware(true); + var docFactory = SecureXmlParserFactory.newDocumentBuilderFactory(true); DocumentBuilder docBuilder = docFactory.newDocumentBuilder(); Document finalDoc = docBuilder.newDocument(); @@ -1176,14 +1155,7 @@ public void warning(org.xml.sax.SAXParseException e) { private boolean isRemediationElementValid(Element remediationElement, FPRInfo fprInfo) { String instanceId = remediationElement.getAttribute("instanceId"); try { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - factory.setXIncludeAware(false); - factory.setExpandEntityReferences(false); - factory.setNamespaceAware(true); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(true); DocumentBuilder builder = factory.newDocumentBuilder(); Document tempDoc = builder.newDocument(); diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/FVDLProcessor.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/FVDLProcessor.java index b1a41b2728c..a875d89b828 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/FVDLProcessor.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/FVDLProcessor.java @@ -45,6 +45,7 @@ import com.fortify.cli.aviator.fpr.utils.XmlUtils; import com.fortify.cli.aviator.util.FprHandle; import com.fortify.cli.aviator.util.StringUtil; +import com.fortify.cli.common.util.SecureXmlParserFactory; import jakarta.xml.bind.JAXBContext; import jakarta.xml.bind.JAXBException; @@ -129,9 +130,7 @@ private FVDL unmarshalFVDL(Path fvdlFilePath) throws JAXBException, IOException try (InputStream fis = Files.newInputStream(fvdlFilePath)) { JAXBContext jaxbContext = JAXBContext.newInstance(FVDL.class); Unmarshaller unmarshaller = jaxbContext.createUnmarshaller(); - javax.xml.stream.XMLInputFactory xmlInputFactory = javax.xml.stream.XMLInputFactory.newInstance(); - xmlInputFactory.setProperty(javax.xml.stream.XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); - xmlInputFactory.setProperty(javax.xml.stream.XMLInputFactory.SUPPORT_DTD, false); + var xmlInputFactory = SecureXmlParserFactory.newXmlInputFactory(); javax.xml.stream.XMLStreamReader xmlStreamReader = xmlInputFactory.createXMLStreamReader(fis); return (FVDL) unmarshaller.unmarshal(xmlStreamReader); } catch (javax.xml.stream.XMLStreamException e) { diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/FilterTemplateParser.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/FilterTemplateParser.java index 07c0d04b716..a6a0a960445 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/FilterTemplateParser.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/FilterTemplateParser.java @@ -25,7 +25,6 @@ import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.transform.OutputKeys; import javax.xml.transform.Transformer; import javax.xml.transform.TransformerFactory; @@ -49,6 +48,7 @@ import com.fortify.cli.aviator.fpr.filter.TagValue; import com.fortify.cli.aviator.util.Constants; import com.fortify.cli.aviator.util.FprHandle; +import com.fortify.cli.common.util.SecureXmlParserFactory; public class FilterTemplateParser { @@ -72,14 +72,7 @@ public Optional parseFilterTemplate() { return Optional.empty(); } - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - factory.setValidating(false); - factory.setNamespaceAware(true); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(true); DocumentBuilder builder = factory.newDocumentBuilder(); diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/StreamingFVDLProcessor.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/StreamingFVDLProcessor.java index c09dbae3599..96c535c4228 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/StreamingFVDLProcessor.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/processor/StreamingFVDLProcessor.java @@ -37,6 +37,7 @@ import com.fortify.cli.aviator.fpr.utils.XmlUtils; import com.fortify.cli.aviator.util.FprHandle; import com.fortify.cli.aviator.util.StringUtil; +import com.fortify.cli.common.util.SecureXmlParserFactory; import lombok.AllArgsConstructor; import lombok.Getter; @@ -84,10 +85,7 @@ public StreamingFVDLProcessor(FprHandle fprHandle, ISourceDecoder sourceDecoder) this.vulnFinalizer = new VulnFinalizer(); this.fprHandle = fprHandle; this.sourceFileMap = fprHandle.getSourceFileMap(); - this.xmlInputFactory = XMLInputFactory.newInstance(); - // Security: Disable external entity processing - xmlInputFactory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); - xmlInputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + this.xmlInputFactory = SecureXmlParserFactory.newXmlInputFactory(); this.fvdlMetadata = new FVDLMetadata(); // Same metadata instance FileUtils will see once encodings are registered during parse. this.fileUtils = new FileUtils( diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/xmlprocessor/RemediationXmlReader.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/xmlprocessor/RemediationXmlReader.java index b81cf5ba87c..d4502fc87b3 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/xmlprocessor/RemediationXmlReader.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/fpr/remediation/xmlprocessor/RemediationXmlReader.java @@ -18,7 +18,6 @@ import java.nio.file.Path; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import org.slf4j.Logger; @@ -27,6 +26,7 @@ import org.xml.sax.SAXException; import com.fortify.cli.aviator._common.exception.AviatorTechnicalException; +import com.fortify.cli.common.util.SecureXmlParserFactory; /** Phase 1: parses {@code remediations.xml} into a DOM {@link Document}. No mapping, no business logic. */ public final class RemediationXmlReader { @@ -34,13 +34,7 @@ public final class RemediationXmlReader { public Document read(Path remediationPath) { try (InputStream remediationStream = Files.newInputStream(remediationPath)) { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - factory.setXIncludeAware(false); - factory.setExpandEntityReferences(false); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(true); DocumentBuilder builder = factory.newDocumentBuilder(); return builder.parse(remediationStream); } catch (ParserConfigurationException | SAXException | IOException e) { diff --git a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/util/FprHandle.java b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/util/FprHandle.java index 4b10fcab4b7..f6d09f98d48 100644 --- a/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/util/FprHandle.java +++ b/fcli-core/fcli-aviator-common/src/main/java/com/fortify/cli/aviator/util/FprHandle.java @@ -24,9 +24,7 @@ import java.util.regex.Pattern; import java.util.stream.Stream; -import javax.xml.XMLConstants; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import org.slf4j.Logger; @@ -38,6 +36,7 @@ import com.fortify.cli.aviator._common.exception.AviatorSimpleException; import com.fortify.cli.aviator._common.exception.AviatorTechnicalException; +import com.fortify.cli.common.util.SecureXmlParserFactory; import lombok.Getter; @@ -195,17 +194,7 @@ private Map loadSourceFileMap() { private void loadEntryXmlFormat(byte[] indexBytes, Map map) throws ParserConfigurationException, IOException, SAXException { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); - factory.setFeature("http://xml.org/sax/features/validation", false); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); - factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, ""); - factory.setXIncludeAware(false); - factory.setExpandEntityReferences(false); + var factory = SecureXmlParserFactory.newDocumentBuilderFactoryWithoutDoctype(false); DocumentBuilder builder = factory.newDocumentBuilder(); Document indexDoc = builder.parse(new ByteArrayInputStream(indexBytes)); diff --git a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/dast/StreamingWebInspectParserTest.java b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/dast/StreamingWebInspectParserTest.java index 582b6641154..59a08131021 100644 --- a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/dast/StreamingWebInspectParserTest.java +++ b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/dast/StreamingWebInspectParserTest.java @@ -13,6 +13,9 @@ package com.fortify.cli.aviator.dast; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import java.nio.charset.StandardCharsets; import java.nio.file.FileSystem; @@ -22,9 +25,12 @@ import java.util.Base64; import java.util.Map; +import javax.xml.stream.XMLStreamException; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import com.fortify.cli.aviator._common.exception.AviatorTechnicalException; import com.fortify.cli.aviator.util.FprHandle; class StreamingWebInspectParserTest { @@ -57,6 +63,25 @@ void parseSessionsPreservesCompleteAuditContext() throws Exception { } } + @Test + void domAndStreamingParsersDoNotResolveExternalEntities() throws Exception { + Path secret = tempDir.resolve("secret.txt"); + Files.writeString(secret, "PRIVATE-SENTINEL"); + Path fpr = createFpr(); + + try (FprHandle handle = new FprHandle(fpr)) { + String maliciousXml = webInspectXml() + .replace("", "]>") + .replace("https://example.test/login", "&xxe;"); + Files.writeString(handle.getPath("/webinspect.xml"), maliciousXml); + + assertNotEquals("PRIVATE-SENTINEL", new WebInspectParser(handle).parseSessions().get(0).getUrl()); + AviatorTechnicalException exception = assertThrows(AviatorTechnicalException.class, + () -> new StreamingWebInspectParser(handle).parseSessions()); + assertInstanceOf(XMLStreamException.class, exception.getCause()); + } + } + private Path createFpr() throws Exception { Path fpr = tempDir.resolve("dast.fpr"); try (FileSystem zip = FileSystems.newFileSystem(fpr, Map.of("create", "true"))) { diff --git a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/model/FPRInfoTest.java b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/model/FPRInfoTest.java index 0c08d872306..0c00ae9271f 100644 --- a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/model/FPRInfoTest.java +++ b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/model/FPRInfoTest.java @@ -12,6 +12,8 @@ */ package com.fortify.cli.aviator.fpr.model; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -22,6 +24,8 @@ import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; +import javax.xml.stream.XMLStreamException; + import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -46,6 +50,38 @@ void throwsTechnicalExceptionWhenAuditFvdlIsMissing() throws Exception { } } + @Test + void allowsHarmlessDoctypeInAuditFvdl() throws Exception { + Path fprPath = createFpr(""" + + safe-uuidsafe-build + 712 + """); + + try (FprHandle fprHandle = new FprHandle(fprPath)) { + var metadata = new FPRInfo(fprHandle); + assertEquals("safe-uuid", metadata.getUuid()); + assertEquals("safe-build", metadata.getBuildId()); + assertEquals(7, metadata.getNumberOfFiles()); + assertEquals(12, metadata.getScanTime()); + } + } + + @Test + void doesNotResolveExternalEntityInAuditFvdl() throws Exception { + Path secret = tempDir.resolve("secret.txt"); + Files.writeString(secret, "PRIVATE-SENTINEL"); + Path fprPath = createFpr(""" + ]> + &xxe; + """.formatted(secret.toUri())); + + try (FprHandle fprHandle = new FprHandle(fprPath)) { + AviatorTechnicalException exception = assertThrows(AviatorTechnicalException.class, () -> new FPRInfo(fprHandle)); + assertInstanceOf(XMLStreamException.class, exception.getCause()); + } + } + private Path createFpr(String auditFvdlContent) throws IOException { Path fprPath = tempDir.resolve("test.fpr"); try (ZipOutputStream zipOutputStream = new ZipOutputStream(Files.newOutputStream(fprPath))) { diff --git a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/AuditProcessorAuditIsolationTest.java b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/AuditProcessorAuditIsolationTest.java index 21f1bf0a985..5f3137c29bd 100644 --- a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/AuditProcessorAuditIsolationTest.java +++ b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/AuditProcessorAuditIsolationTest.java @@ -89,6 +89,25 @@ void testPrunesUntouchedIssuesAndKeepsAuditedIssueState() throws Exception { assertTrue(hasTagHistory(audited)); } + @Test + void doesNotResolveExternalEntityInAuditXml() throws Exception { + Path secret = Files.createTempFile("audit-xxe-sentinel", ".txt"); + try { + Files.writeString(secret, "PRIVATE-SENTINEL"); + createTestFpr(""" + ]> + + &xxe; + + """.formatted(secret.toUri())); + + var issue = new AuditProcessor(fprHandle).processAuditXML().get("issue-1"); + assertEquals("", issue.getThreadedComments().get(0).getContent()); + } finally { + Files.deleteIfExists(secret); + } + } + @Test void testPartialSuccessRetainsOnlySuccessfulIssue() throws Exception { createTestFpr(multiIssueAuditXml("instance-A", "instance-D")); diff --git a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/FilterTemplateParserTest.java b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/FilterTemplateParserTest.java index c608f289ead..038892f5e24 100644 --- a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/FilterTemplateParserTest.java +++ b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/FilterTemplateParserTest.java @@ -12,6 +12,7 @@ */ package com.fortify.cli.aviator.fpr.processor; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -47,6 +48,41 @@ void throwsTechnicalExceptionOnMalformedFilterTemplateXml() throws Exception { } } + @Test + void allowsHarmlessDoctypeWithoutLoadingExternalDtd() throws Exception { + String xml = """ + + Safe template + """; + Path fprPath = createFpr(xml); + try (FprHandle fprHandle = new FprHandle(fprPath)) { + var parser = new FilterTemplateParser(fprHandle, new AuditProcessor(fprHandle)); + var template = parser.parseFilterTemplate().orElseThrow(); + assertEquals("Safe template", template.getName()); + assertEquals("template", template.getId()); + assertTrue(!template.getTagDefinitions().isEmpty()); + assertEquals(xml, Files.readString(fprHandle.getPath("/filtertemplate.xml"))); + } + } + + @Test + void doesNotDiscloseExternalEntityInFilterTemplate() throws Exception { + Path secret = tempDir.resolve("secret.txt"); + Files.writeString(secret, "PRIVATE-SENTINEL"); + String xml = """ + ]> + &xxe; + """.formatted(secret.toUri()); + Path fprPath = createFpr(xml); + + try (FprHandle fprHandle = new FprHandle(fprPath)) { + FilterTemplateParser parser = new FilterTemplateParser(fprHandle, new AuditProcessor(fprHandle)); + var template = parser.parseFilterTemplate().orElseThrow(); + assertEquals("", template.getName()); + assertEquals(xml, Files.readString(fprHandle.getPath("/filtertemplate.xml"))); + } + } + private Path createFpr(String filterTemplateXml) throws IOException { Path fprPath = tempDir.resolve("test.fpr"); try (ZipOutputStream zipOutputStream = new ZipOutputStream(Files.newOutputStream(fprPath))) { diff --git a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/StreamingFVDLProcessorTest.java b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/StreamingFVDLProcessorTest.java index acd403c8c49..5e6f905b57e 100644 --- a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/StreamingFVDLProcessorTest.java +++ b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/processor/StreamingFVDLProcessorTest.java @@ -13,9 +13,13 @@ package com.fortify.cli.aviator.fpr.processor; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; @@ -23,6 +27,8 @@ import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; +import javax.xml.stream.XMLStreamException; + import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; @@ -75,6 +81,27 @@ void testParseUsesRunEngineNameAndIgnoresEngineDataPropertiesForAnalysisType() t assertEquals("PTA", processor.getFvdlMetadata().getAnalysisType()); } + @Test + void testParseDoesNotResolveExternalEntity() throws Exception { + Path secret = Files.createTempFile("fvdl-xxe-sentinel", ".txt"); + try { + Files.writeString(secret, "PRIVATE-SENTINEL"); + createTestFpr(""" + ]> + &xxe; + """.formatted(secret.toUri())); + + StreamingFVDLProcessor processor = new StreamingFVDLProcessor(fprHandle); + try (ZipFile zipFile = new ZipFile(tempFprFile.toFile())) { + IOException exception = assertThrows(IOException.class, () -> processor.parse(zipFile, "audit.fvdl")); + assertInstanceOf(XMLStreamException.class, exception.getCause()); + assertTrue(!exception.getMessage().contains("PRIVATE-SENTINEL")); + } + } finally { + Files.deleteIfExists(secret); + } + } + @Test void testParsePreservesRuleMetadataWithoutTracesAndKeepsDefaultAnalysisType() throws Exception { String xml = """ diff --git a/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/remediation/xmlprocessor/RemediationXmlReaderTest.java b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/remediation/xmlprocessor/RemediationXmlReaderTest.java new file mode 100644 index 00000000000..ffd06462977 --- /dev/null +++ b/fcli-core/fcli-aviator-common/src/test/java/com/fortify/cli/aviator/fpr/remediation/xmlprocessor/RemediationXmlReaderTest.java @@ -0,0 +1,52 @@ +/* + * Copyright 2021-2026 Open Text. + * + * The only warranties for products and services of Open Text + * and its affiliates and licensors ("Open Text") are as may + * be set forth in the express warranty statements accompanying + * such products and services. Nothing herein should be construed + * as constituting an additional warranty. Open Text shall not be + * liable for technical or editorial errors or omissions contained + * herein. The information contained herein is subject to change + * without notice. + */ +package com.fortify.cli.aviator.fpr.remediation.xmlprocessor; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +class RemediationXmlReaderTest { + @TempDir + Path tempDir; + + @Test + void doesNotResolveExternalEntity() throws Exception { + Path secret = tempDir.resolve("secret.txt"); + Files.writeString(secret, "PRIVATE-SENTINEL"); + Path remediations = tempDir.resolve("remediations.xml"); + Files.writeString(remediations, """ + ]> + &xxe; + """.formatted(secret.toUri())); + + var document = new RemediationXmlReader().read(remediations); + assertEquals("", document.getElementsByTagName("Comment").item(0).getTextContent()); + } + + @Test + void allowsDoctypeWithoutLoadingExternalDtd() throws Exception { + Path remediations = tempDir.resolve("remediations.xml"); + Files.writeString(remediations, """ + + expected + """); + + var document = new RemediationXmlReader().read(remediations); + assertEquals("expected", document.getElementsByTagName("Comment").item(0).getTextContent()); + } +} \ No newline at end of file diff --git a/fcli-core/fcli-aviator/src/main/java/com/fortify/cli/aviator/ssc/helper/DastFprCorrelationEnricher.java b/fcli-core/fcli-aviator/src/main/java/com/fortify/cli/aviator/ssc/helper/DastFprCorrelationEnricher.java index 670eb9f4f4f..bac2e5c5d55 100644 --- a/fcli-core/fcli-aviator/src/main/java/com/fortify/cli/aviator/ssc/helper/DastFprCorrelationEnricher.java +++ b/fcli-core/fcli-aviator/src/main/java/com/fortify/cli/aviator/ssc/helper/DastFprCorrelationEnricher.java @@ -12,6 +12,7 @@ */ package com.fortify.cli.aviator.ssc.helper; +import java.io.InputStream; import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; @@ -26,7 +27,7 @@ import java.util.Map; import java.util.Set; -import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.XMLConstants; import javax.xml.transform.OutputKeys; import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; @@ -40,6 +41,7 @@ import com.fortify.cli.aviator.grpc.CorrelatedPair; import com.fortify.cli.aviator.util.FprHandle; +import com.fortify.cli.common.util.SecureXmlParserFactory; import lombok.SneakyThrows; @@ -167,25 +169,19 @@ private Map> groupByDastId(List pai @SneakyThrows private Document parseXml(Path path) { - var factory = DocumentBuilderFactory.newInstance(); - // Disable DOCTYPE declarations and external entity processing to prevent XXE attacks - try { - factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); - factory.setFeature("http://xml.org/sax/features/external-general-entities", false); - factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - factory.setXIncludeAware(false); - factory.setExpandEntityReferences(false); - } catch (Exception e) { - LOG.warn("Could not configure XXE protection for DocumentBuilderFactory; some protections may be unavailable: {}", - e.getMessage()); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(false); + try (InputStream input = Files.newInputStream(path)) { + return factory.newDocumentBuilder().parse(input); } - factory.setNamespaceAware(false); - return factory.newDocumentBuilder().parse(Files.newInputStream(path)); } @SneakyThrows private void writeXml(Document doc, Path path) { - var transformer = TransformerFactory.newInstance().newTransformer(); + var factory = TransformerFactory.newInstance(); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); + var transformer = factory.newTransformer(); transformer.setOutputProperty(OutputKeys.INDENT, "yes"); transformer.setOutputProperty(OutputKeys.ENCODING, "UTF-8"); transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2"); diff --git a/fcli-core/fcli-aviator/src/main/java/com/fortify/cli/aviator/ssc/helper/SastFprCorrelationRecorder.java b/fcli-core/fcli-aviator/src/main/java/com/fortify/cli/aviator/ssc/helper/SastFprCorrelationRecorder.java index 6d6364457b3..a704e1815e8 100644 --- a/fcli-core/fcli-aviator/src/main/java/com/fortify/cli/aviator/ssc/helper/SastFprCorrelationRecorder.java +++ b/fcli-core/fcli-aviator/src/main/java/com/fortify/cli/aviator/ssc/helper/SastFprCorrelationRecorder.java @@ -12,6 +12,7 @@ */ package com.fortify.cli.aviator.ssc.helper; +import java.io.InputStream; import java.io.OutputStream; import java.nio.file.Files; import java.nio.file.Path; @@ -23,7 +24,7 @@ import java.util.Set; import java.util.stream.Collectors; -import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.XMLConstants; import javax.xml.transform.OutputKeys; import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; @@ -37,6 +38,7 @@ import com.fortify.cli.aviator.grpc.CorrelatedPair; import com.fortify.cli.aviator.util.FprHandle; +import com.fortify.cli.common.util.SecureXmlParserFactory; import lombok.SneakyThrows; @@ -369,14 +371,19 @@ private static void upsertCorrelationTag(Document doc, Element issue, String val @SneakyThrows private static Document parseXml(Path path) { - var factory = DocumentBuilderFactory.newInstance(); - factory.setNamespaceAware(true); // must be true to find ns0:Issue / ns0:Tag by local name - return factory.newDocumentBuilder().parse(Files.newInputStream(path)); + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(true); + try (InputStream input = Files.newInputStream(path)) { + return factory.newDocumentBuilder().parse(input); + } } @SneakyThrows private static void writeXml(Document doc, Path path) { - var transformer = TransformerFactory.newInstance().newTransformer(); + var factory = TransformerFactory.newInstance(); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); + factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); + var transformer = factory.newTransformer(); transformer.setOutputProperty(OutputKeys.INDENT, "yes"); transformer.setOutputProperty(OutputKeys.ENCODING, "UTF-8"); transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2"); diff --git a/fcli-core/fcli-aviator/src/main/resources/com/fortify/cli/aviator/i18n/AviatorMessages.properties b/fcli-core/fcli-aviator/src/main/resources/com/fortify/cli/aviator/i18n/AviatorMessages.properties index 84143ca3147..53db9c72176 100644 --- a/fcli-core/fcli-aviator/src/main/resources/com/fortify/cli/aviator/i18n/AviatorMessages.properties +++ b/fcli-core/fcli-aviator/src/main/resources/com/fortify/cli/aviator/i18n/AviatorMessages.properties @@ -209,9 +209,8 @@ fcli.aviator.ssc.apply-remediations.usage.description = Downloads FPR artifact(s %n fcli aviator ssc apply-remediations --av "MyApp:1.0" --latest --source-dir /path/to/src fcli.aviator.ssc.apply-remediations.from-cache = Local remediations cache zip produced by download-remediations-cache. Mutually exclusive with online selection options. Does not require an SSC session. fcli.aviator.ssc.apply-remediations.source-dir = Source code directory where remediations will be applied. Defaults to current directory. -fcli.aviator.ssc.apply-remediations.preview = Dry-run mode: validates all remediations, reads source files for verification, and outputs detailed change information without modifying any files. \ - Performs full processing including file reading, encoding validation, and change preparation. Output includes a previewDetails array with per-issue change information. \ - Compatible with both --from-cache (offline) and online artifact selection. Works with --issue-ids to preview specific remediations. +fcli.aviator.ssc.apply-remediations.preview = Shows available remediations and their proposed changes without modifying source files. \ + Does not validate whether the proposed changes apply cleanly to the current source. fcli.aviator.ssc.apply-remediations.issue-ids = Comma-separated list of issue IDs to apply. Matches requested values against remediations.xml \ instanceId values. Requires --from-cache so integrations can download once via download-remediations-cache and apply selected remediations without repeated SSC downloads. # Shared by download-remediations-cache and apply-remediations online selection (AviatorSSCRemediationsSelectorArgGroups). diff --git a/fcli-core/fcli-aviator/src/test/java/com/fortify/cli/aviator/ssc/helper/DastFprCorrelationEnricherTest.java b/fcli-core/fcli-aviator/src/test/java/com/fortify/cli/aviator/ssc/helper/DastFprCorrelationEnricherTest.java index 1fcd6e8b364..8970cf2aa7f 100644 --- a/fcli-core/fcli-aviator/src/test/java/com/fortify/cli/aviator/ssc/helper/DastFprCorrelationEnricherTest.java +++ b/fcli-core/fcli-aviator/src/test/java/com/fortify/cli/aviator/ssc/helper/DastFprCorrelationEnricherTest.java @@ -13,6 +13,8 @@ package com.fortify.cli.aviator.ssc.helper; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import java.nio.charset.StandardCharsets; import java.nio.file.FileSystem; @@ -26,6 +28,8 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import com.fortify.cli.aviator.grpc.CorrelatedPair; import com.fortify.cli.aviator.util.FprHandle; @@ -33,9 +37,17 @@ class DastFprCorrelationEnricherTest { @TempDir Path tempDir; - @Test - void preservesExistingExternalFindingsAndAvoidsDuplicates() throws Exception { + @ParameterizedTest + @ValueSource(booleans = { false, true }) + void preservesExistingExternalFindingsAndAvoidsDuplicates(boolean withDoctype) throws Exception { Path fprPath = createFpr(); + if (withDoctype) { + try (FprHandle fprHandle = new FprHandle(fprPath)) { + Path xmlPath = fprHandle.getPath("/webinspect.xml"); + Files.writeString(xmlPath, "" + + Files.readString(xmlPath)); + } + } var pairs = List.of( new CorrelatedPair("SAST-1", "DAST-1", "scan-1", "HIGH", "existing"), new CorrelatedPair("SAST-2", "DAST-1", "scan-2", "HIGH", "new") @@ -55,6 +67,35 @@ void preservesExistingExternalFindingsAndAvoidsDuplicates() throws Exception { } } + @Test + void doesNotDiscloseExternalEntityInWebInspectXml() throws Exception { + Path secret = tempDir.resolve("secret.txt"); + Files.writeString(secret, "PRIVATE-SENTINEL"); + Path fprPath = createFpr(); + String maliciousXml = """ + ]> + &xxe; + """.formatted(secret.toUri()); + try (FprHandle fprHandle = new FprHandle(fprPath)) { + Files.writeString(fprHandle.getPath("/webinspect.xml"), maliciousXml); + } + + new DastFprCorrelationEnricher().injectAndRepackage(fprPath, + List.of(new CorrelatedPair("SAST-1", "DAST-1", "scan-1", "HIGH", "match"))); + try (FprHandle fprHandle = new FprHandle(fprPath)) { + String output = Files.readString(fprHandle.getPath("/webinspect.xml")); + assertFalse(output.contains("PRIVATE-SENTINEL")); + try (var input = Files.newInputStream(fprHandle.getPath("/webinspect.xml"))) { + var document = DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(input); + var originFindingIds = document.getElementsByTagName("OriginFindingID"); + assertEquals(2, originFindingIds.getLength()); + assertEquals("", originFindingIds.item(0).getTextContent()); + assertEquals("SAST-1", originFindingIds.item(1).getTextContent()); + assertTrue(output.contains("AI_CORRELATION_METADATA")); + } + } + } + private Path createFpr() throws Exception { Path fprPath = tempDir.resolve("merged.fpr"); try (FileSystem zipFs = FileSystems.newFileSystem(fprPath, Map.of("create", "true"))) { diff --git a/fcli-core/fcli-aviator/src/test/java/com/fortify/cli/aviator/ssc/helper/SastFprCorrelationRecorderTest.java b/fcli-core/fcli-aviator/src/test/java/com/fortify/cli/aviator/ssc/helper/SastFprCorrelationRecorderTest.java index 619ab9d3863..e382c895882 100644 --- a/fcli-core/fcli-aviator/src/test/java/com/fortify/cli/aviator/ssc/helper/SastFprCorrelationRecorderTest.java +++ b/fcli-core/fcli-aviator/src/test/java/com/fortify/cli/aviator/ssc/helper/SastFprCorrelationRecorderTest.java @@ -27,6 +27,7 @@ import org.junit.jupiter.api.io.TempDir; import com.fortify.cli.aviator.grpc.CorrelatedPair; +import com.fortify.cli.aviator.util.FprHandle; class SastFprCorrelationRecorderTest { @@ -181,8 +182,67 @@ void testReadTriedPairKeys_noAuditXml() throws Exception { assertTrue(keys.isEmpty()); } + @Test + void testReadTriedPairKeys_doesNotResolveExternalEntity() throws Exception { + Path secret = tempDir.resolve("secret.txt"); + Files.writeString(secret, "PRIVATE-SENTINEL"); + Path fprPath = createFprWithAuditXml(""" + + ]> + + CORRELATED::&xxe; + + """.formatted(secret.toUri(), SastFprCorrelationRecorder.DAST_CORRELATION_TAG_ID)); + + assertTrue(SastFprCorrelationRecorder.readTriedPairKeys(fprPath).isEmpty()); + SastFprCorrelationRecorder.writeCorrelationTags(fprPath, + List.of(new CorrelatedPair("SAST-1", "DAST-A", SCAN_GUID, "HIGH", "match")), List.of()); + try (FileSystem zipFs = FileSystems.newFileSystem(fprPath)) { + String output = Files.readString(zipFs.getPath("/audit.xml")); + assertTrue(output.contains("CORRELATED::DAST-A")); + assertTrue(!output.contains("PRIVATE-SENTINEL")); + } + } + + @Test + void testReadTriedPairKeys_allowsHarmlessDoctype() throws Exception { + Path fprPath = createFprWithAuditXml(""" + + + + CORRELATED::DAST-A + + """.formatted(SastFprCorrelationRecorder.DAST_CORRELATION_TAG_ID)); + + try (FprHandle handle = new FprHandle(fprPath)) { + Files.createDirectories(handle.getPath("/src-archive")); + Files.writeString(handle.getPath("/src-archive/index.xml"), """ + + + src-archive/Test.java + """); + Files.writeString(handle.getPath("/src-archive/Test.java"), "class Test {}"); + assertEquals("src-archive/Test.java", handle.getSourceFileMap().get("Test.java")); + } + + SastFprCorrelationRecorder.writeCorrelationTags(fprPath, + List.of(new CorrelatedPair("SAST-1", "DAST-B", SCAN_GUID, "HIGH", "match")), List.of()); + assertEquals(Set.of("SAST-1::DAST-A", "SAST-1::DAST-B"), SastFprCorrelationRecorder.readTriedPairKeys(fprPath)); + try (FprHandle handle = new FprHandle(fprPath)) { + assertEquals("src-archive/Test.java", handle.getSourceFileMap().get("Test.java")); + } + } + // ── helpers ────────────────────────────────────────────────────────── + private Path createFprWithAuditXml(String auditXml) throws Exception { + Path fprPath = tempDir.resolve("custom-audit.fpr"); + try (FileSystem zipFs = FileSystems.newFileSystem(fprPath, Map.of("create", "true"))) { + Files.writeString(zipFs.getPath("/audit.xml"), auditXml); + } + return fprPath; + } + private Path createMinimalSastFpr() throws Exception { Path fprPath = tempDir.resolve("test-sast.fpr"); if (Files.exists(fprPath)) { diff --git a/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/util/SecureXmlParserFactory.java b/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/util/SecureXmlParserFactory.java new file mode 100644 index 00000000000..436016c17dd --- /dev/null +++ b/fcli-core/fcli-common-core/src/main/java/com/fortify/cli/common/util/SecureXmlParserFactory.java @@ -0,0 +1,56 @@ +/* + * Copyright 2021-2026 Open Text. + * + * The only warranties for products and services of Open Text + * and its affiliates and licensors ("Open Text") are as may + * be set forth in the express warranty statements accompanying + * such products and services. Nothing herein should be construed + * as constituting an additional warranty. Open Text shall not be + * liable for technical or editorial errors or omissions contained + * herein. The information contained herein is subject to change + * without notice. + */ +package com.fortify.cli.common.util; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; +import javax.xml.stream.XMLInputFactory; + +public final class SecureXmlParserFactory { + private SecureXmlParserFactory() {} + + /** Creates a fresh DOM factory that permits DOCTYPE declarations without resolving external resources. */ + public static DocumentBuilderFactory newDocumentBuilderFactory(boolean namespaceAware) + throws ParserConfigurationException { + return newDocumentBuilderFactory(namespaceAware, false); + } + + /** Creates a fresh DOM factory that rejects all DOCTYPE declarations. */ + public static DocumentBuilderFactory newDocumentBuilderFactoryWithoutDoctype(boolean namespaceAware) + throws ParserConfigurationException { + return newDocumentBuilderFactory(namespaceAware, true); + } + + private static DocumentBuilderFactory newDocumentBuilderFactory(boolean namespaceAware, boolean disallowDoctype) + throws ParserConfigurationException { + var factory = DocumentBuilderFactory.newInstance(); + factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", disallowDoctype); + factory.setFeature("http://xml.org/sax/features/external-general-entities", false); + factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + factory.setXIncludeAware(false); + factory.setExpandEntityReferences(false); + factory.setNamespaceAware(namespaceAware); + return factory; + } + + /** Creates a fresh streaming factory with DTD processing and external entity support disabled. */ + public static XMLInputFactory newXmlInputFactory() { + var factory = XMLInputFactory.newInstance(); + factory.setProperty(XMLInputFactory.SUPPORT_DTD, false); + factory.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false); + return factory; + } +} \ No newline at end of file diff --git a/fcli-core/fcli-common-core/src/test/java/com/fortify/cli/common/util/SecureXmlParserFactoryTest.java b/fcli-core/fcli-common-core/src/test/java/com/fortify/cli/common/util/SecureXmlParserFactoryTest.java new file mode 100644 index 00000000000..ba1a89c39ed --- /dev/null +++ b/fcli-core/fcli-common-core/src/test/java/com/fortify/cli/common/util/SecureXmlParserFactoryTest.java @@ -0,0 +1,126 @@ +/* + * Copyright 2021-2026 Open Text. + * + * The only warranties for products and services of Open Text + * and its affiliates and licensors ("Open Text") are as may + * be set forth in the express warranty statements accompanying + * such products and services. Nothing herein should be construed + * as constituting an additional warranty. Open Text shall not be + * liable for technical or editorial errors or omissions contained + * herein. The information contained herein is subject to change + * without notice. + */ +package com.fortify.cli.common.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.StringReader; + +import javax.xml.XMLConstants; +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.stream.XMLInputFactory; +import javax.xml.stream.XMLStreamException; + +import org.junit.jupiter.api.Test; +import org.w3c.dom.Document; +import org.xml.sax.InputSource; +import org.xml.sax.SAXException; + +class SecureXmlParserFactoryTest { + @Test + void domUsesSecurePolicyAndFreshFactories() throws Exception { + var factory = SecureXmlParserFactory.newDocumentBuilderFactory(true); + assertNotSame(factory, SecureXmlParserFactory.newDocumentBuilderFactory(true)); + assertFalse(factory.getFeature("http://apache.org/xml/features/disallow-doctype-decl")); + assertFalse(factory.getFeature("http://xml.org/sax/features/external-general-entities")); + assertFalse(factory.getFeature("http://xml.org/sax/features/external-parameter-entities")); + assertFalse(factory.getFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd")); + assertTrue(factory.getFeature(XMLConstants.FEATURE_SECURE_PROCESSING)); + assertFalse(factory.isXIncludeAware()); + assertFalse(factory.isExpandEntityReferences()); + assertTrue(factory.isNamespaceAware()); + } + + @Test + void domAllowsHarmlessDoctypeAndPreservesNamespaceChoice() throws Exception { + var xml = "safe"; + var aware = parse(SecureXmlParserFactory.newDocumentBuilderFactory(true), xml); + assertEquals("urn:test", aware.getDocumentElement().getNamespaceURI()); + assertEquals("safe", aware.getDocumentElement().getTextContent()); + var unaware = parse(SecureXmlParserFactory.newDocumentBuilderFactory(false), xml); + assertNull(unaware.getDocumentElement().getNamespaceURI()); + } + + @Test + void domDoesNotResolveExternalEntitiesOrDtd() throws Exception { + for (var declaration : new String[] { + "", + "]>", + "%external;]>" }) { + var body = declaration.contains("" + body + ""); + assertEquals(body.equals("safe") ? "safe" : "", document.getDocumentElement().getTextContent()); + } + } + + @Test + void strictDomRejectsDoctype() throws Exception { + var factory = SecureXmlParserFactory.newDocumentBuilderFactoryWithoutDoctype(false); + assertThrows(SAXException.class, () -> parse(factory, "")); + } + + @Test + void staxDoesNotResolveExternalDtdAndPreservesNamespaces() throws Exception { + var factory = SecureXmlParserFactory.newXmlInputFactory(); + assertNotSame(factory, SecureXmlParserFactory.newXmlInputFactory()); + assertEquals(false, factory.getProperty(XMLInputFactory.SUPPORT_DTD)); + assertEquals(false, factory.getProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES)); + factory.setXMLResolver((publicId, systemId, baseUri, namespace) -> { + throw new AssertionError("External XML resource requested"); + }); + var reader = factory.createXMLStreamReader(new StringReader( + "safe")); + try { + while (reader.hasNext() && !reader.isStartElement()) { + reader.next(); + } + assertEquals("urn:test", reader.getNamespaceURI()); + assertEquals("safe", reader.getElementText()); + } finally { + reader.close(); + } + } + + @Test + void staxDoesNotResolveExternalEntity() throws Exception { + var factory = SecureXmlParserFactory.newXmlInputFactory(); + factory.setXMLResolver((publicId, systemId, baseUri, namespace) -> { + throw new AssertionError("External XML resource requested"); + }); + var reader = factory.createXMLStreamReader(new StringReader( + "]>&external;")); + try { + assertThrows(XMLStreamException.class, () -> { + while (reader.hasNext()) { + reader.next(); + } + }); + } finally { + reader.close(); + } + } + + private Document parse(DocumentBuilderFactory factory, String xml) throws Exception { + var builder = factory.newDocumentBuilder(); + builder.setEntityResolver((publicId, systemId) -> { + throw new AssertionError("External XML resource requested"); + }); + return builder.parse(new InputSource(new StringReader(xml))); + } +} \ No newline at end of file diff --git a/fcli-core/fcli-fod/src/main/resources/com/fortify/cli/fod/i18n/FoDMessages.properties b/fcli-core/fcli-fod/src/main/resources/com/fortify/cli/fod/i18n/FoDMessages.properties index 62a79be9e90..08b3f273c72 100644 --- a/fcli-core/fcli-fod/src/main/resources/com/fortify/cli/fod/i18n/FoDMessages.properties +++ b/fcli-core/fcli-fod/src/main/resources/com/fortify/cli/fod/i18n/FoDMessages.properties @@ -1049,9 +1049,8 @@ fcli.fod.aviator.apply-remediations.usage.description = Downloads the FPR from a Exactly one of --release/--rel or --from-cache must be specified. Online selection requires an FoD session; --from-cache does not. fcli.fod.aviator.apply-remediations.source-dir = Directory containing source code to apply remediations to. Default value: ${DEFAULT-VALUE}. fcli.fod.aviator.apply-remediations.from-cache = Local remediations cache zip produced by download-remediations-cache. Mutually exclusive with --release/--rel. Does not require an FoD session. -fcli.fod.aviator.apply-remediations.preview = Dry-run mode: validates all remediations, reads source files for verification, and outputs detailed change information without modifying any files. \ - Performs full processing including file reading, encoding validation, and change preparation. Output includes a previewDetails array with per-issue change information. \ - Compatible with both --from-cache (offline) and online release selection. Works with --issue-ids to preview specific remediations. +fcli.fod.aviator.apply-remediations.preview = Shows available remediations and their proposed changes without modifying source files. \ + Does not validate whether the proposed changes apply cleanly to the current source. fcli.fod.aviator.apply-remediations.issue-ids = Comma-separated list of issue IDs to apply. Matches requested values against remediations.xml \ instanceId entries. Requires --from-cache so integrations can download once via download-remediations-cache and apply selected remediations without repeated FoD downloads. fcli.fod.aviator.download-remediations-cache.usage.header = Download a remediations cache zip containing Fortify Remediation Aviator remediations from FoD.