From 23c14461290bfb89aa0fbcf566f07c8be42d9830 Mon Sep 17 00:00:00 2001 From: azerr Date: Tue, 18 May 2021 11:37:36 +0200 Subject: [PATCH] Mitigate Billion Laughs vulnerability Fixes https://github.com/redhat-developer/vscode-xml/issues/476 Signed-off-by: azerr --- .../participants/DTDErrorCode.java | 3 + .../LSPXMLParserConfiguration.java | 36 +++++++++- .../contentmodel/DTDDiagnosticsTest.java | 67 +++++++++++++++++++ 3 files changed, 105 insertions(+), 1 deletion(-) diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/DTDErrorCode.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/DTDErrorCode.java index dd5803619d..9ca2845b4e 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/DTDErrorCode.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/DTDErrorCode.java @@ -20,6 +20,7 @@ import org.apache.xerces.xni.XMLLocator; import org.eclipse.lemminx.commons.BadLocationException; import org.eclipse.lemminx.dom.DOMDocument; +import org.eclipse.lemminx.dom.DOMDocumentType; import org.eclipse.lemminx.dom.DOMElement; import org.eclipse.lemminx.dom.DOMRange; import org.eclipse.lemminx.extensions.contentmodel.participants.codeactions.ElementDeclUnterminatedCodeAction; @@ -48,6 +49,7 @@ public enum DTDErrorCode implements IXMLErrorCode { ElementDeclUnterminated, // EntityDeclUnterminated, // EntityNotDeclared, // + EntityExpansionLimitExceeded, // ExternalIDorPublicIDRequired, // IDInvalidWithNamespaces, // IDREFInvalidWithNamespaces, // @@ -170,6 +172,7 @@ public static Range toLSPRange(XMLLocator location, DTDErrorCode code, Object[] case PEReferenceWithinMarkup: { return XMLPositionUtility.getLastValidDTDDeclParameter(offset, document, true); } + case EntityExpansionLimitExceeded: case EntityNotDeclared: { EntityReferenceRange range = XMLPositionUtility.selectEntityReference(offset - 1, document); return range != null ? range.getRange() : null; diff --git a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java index 714b0b9ec9..ba7e020387 100644 --- a/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java +++ b/org.eclipse.lemminx/src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java @@ -11,7 +11,12 @@ *******************************************************************************/ package org.eclipse.lemminx.extensions.contentmodel.participants.diagnostics; +import java.util.logging.Level; +import java.util.logging.Logger; + +import org.apache.xerces.impl.Constants; import org.apache.xerces.impl.dtd.XMLDTDValidator; +import org.apache.xerces.util.SecurityManager; import org.apache.xerces.xni.XMLDocumentHandler; import org.apache.xerces.xni.XNIException; import org.apache.xerces.xni.grammars.XMLGrammarPool; @@ -37,6 +42,16 @@ */ class LSPXMLParserConfiguration extends XMLModelAwareParserConfiguration { + private static final Logger LOGGER = Logger.getLogger(LSPXMLParserConfiguration.class.getName()); + + /** property identifier: security manager. */ + private static final String SECURITY_MANAGER = Constants.XERCES_PROPERTY_PREFIX + + Constants.SECURITY_MANAGER_PROPERTY; + private static final String ENTITY_EXPANSION_LIMIT_PROPERTY_NAME = "jdk.xml.entityExpansionLimit"; + private static final String MAX_OCCUR_LIMIT_PROPERTY_NAME = "jdk.xml.maxOccur"; + private static final int ENTITY_EXPANSION_LIMIT_DEFAULT_VALUE = 64000; + private static final int MAX_OCCUR_LIMIT_DEFAULT_VALUE = 5000; + private final boolean disableDTDValidation; private ExternalXMLDTDValidator externalDTDValidator; @@ -53,6 +68,15 @@ public LSPXMLParserConfiguration(XMLGrammarPool grammarPool, boolean disableDTDV : false; super.setFeature("http://xml.org/sax/features/external-general-entities", resolveExternalEntities); super.setFeature("http://xml.org/sax/features/external-parameter-entities", resolveExternalEntities); + if (resolveExternalEntities) { + // Security manager + SecurityManager securityManager = new SecurityManager(); + securityManager.setEntityExpansionLimit( + getPropertyValue(ENTITY_EXPANSION_LIMIT_PROPERTY_NAME, ENTITY_EXPANSION_LIMIT_DEFAULT_VALUE)); + securityManager.setMaxOccurNodeLimit( + getPropertyValue(MAX_OCCUR_LIMIT_PROPERTY_NAME, MAX_OCCUR_LIMIT_DEFAULT_VALUE)); + super.setProperty(SECURITY_MANAGER, securityManager); + } fErrorReporter = reporterForXML; } @@ -141,7 +165,17 @@ private void configureExternalDTDPipeline() { // in the case of schema have some error (ex : syntax error) AbstractLSPErrorReporter.initializeReporter(fSchemaValidator, getReporterForGrammar()); } - } + private static int getPropertyValue(String propertyName, int defaultValue) { + String value = System.getProperty(propertyName, ""); + if (!value.isEmpty()) { + try { + return Integer.parseInt(value); + } catch (Exception e) { + LOGGER.log(Level.WARNING, "Error while getting system property '" + propertyName + "'.", e); + } + } + return defaultValue; + } } diff --git a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java index dfd234ac8f..1bf76578d4 100644 --- a/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java +++ b/org.eclipse.lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java @@ -767,6 +767,73 @@ public void diagnosticRelatedInformationWithXMLModel() throws Exception { diagnostic, diagnosticBasedOnDTD); } + @Test + public void defaultEntityExpansionLimit() { + ContentModelSettings settings = new ContentModelSettings(); + settings.setUseCache(true); + XMLValidationSettings validationSettings = new XMLValidationSettings(); + validationSettings.setResolveExternalEntities(true); + settings.setValidation(validationSettings); + + String xml = "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + "]>\r\n" + // + "&lol9;"; + + Diagnostic diagnostic = d(14, 6, 14, 12, DTDErrorCode.EntityExpansionLimitExceeded, // + "The parser has encountered more than \"64 000\" entity expansions in this document; this is the limit imposed by the application.", + "xml", DiagnosticSeverity.Error); + XMLAssert.testDiagnosticsFor(new XMLLanguageService(), xml, null, null, null, false, settings, diagnostic); + } + + @Test + public void customEntityExpansionLimit() { + ContentModelSettings settings = new ContentModelSettings(); + settings.setUseCache(true); + XMLValidationSettings validationSettings = new XMLValidationSettings(); + validationSettings.setResolveExternalEntities(true); + settings.setValidation(validationSettings); + + try { + System.setProperty("jdk.xml.entityExpansionLimit", "10"); + + String xml = "\r\n" + // + "\r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + " \r\n" + // + "]>\r\n" + // + "&lol9;"; + + Diagnostic diagnostic = d(14, 6, 14, 12, DTDErrorCode.EntityExpansionLimitExceeded, // + "The parser has encountered more than \"10\" entity expansions in this document; this is the limit imposed by the application.", + "xml", DiagnosticSeverity.Error); + XMLAssert.testDiagnosticsFor(new XMLLanguageService(), xml, null, null, null, false, settings, diagnostic); + + } + finally { + System.setProperty("jdk.xml.entityExpansionLimit", ""); + } + } private static void testDiagnosticsFor(String xml, Diagnostic... expected) { XMLAssert.testDiagnosticsFor(xml, "src/test/resources/catalogs/catalog.xml", expected); }