From 0fc4ff26db10d843e29168b3ef2b932efc2d88aa 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 | 34 +++++++- .../contentmodel/DTDDiagnosticsTest.java | 77 +++++++++++++++++++ 3 files changed, 113 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 dd5803619..9ca2845b4 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 714b0b9ec..e7499c9b4 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,13 @@ 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); + // 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 +163,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 dfd234ac8..d753dbb76 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 @@ -20,6 +20,7 @@ import static org.eclipse.lemminx.XMLAssert.testCodeActionsFor; import java.util.ArrayList; +import java.util.Locale; import org.apache.xerces.impl.XMLEntityManager; import org.apache.xerces.util.URI.MalformedURIException; @@ -767,6 +768,82 @@ 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); + + Locale defaultLocale = Locale.getDefault(); + try { + // Set local as English for formatting integer in error message with ',' + // See 64,000 in "The parser has encountered more than \"64,000\" entity + // expansions in this document; this is the limit imposed by the application." + Locale.setDefault(Locale.ENGLISH); + 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); + } finally { + Locale.setDefault(defaultLocale); + } + } + + @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); }