Skip to content

Commit

Permalink
Mitigate Billion Laughs vulnerability
Browse files Browse the repository at this point in the history 
Fixes redhat-developer/vscode-xml#476

Signed-off-by: azerr <[email protected]>
  • Loading branch information
@angelozerr @datho7561
angelozerr authored and datho7561 committed May 18, 2021
1 parent 3e3bfc1 commit 8c4f23f
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .../src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/DTDErrorCode.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -48,6 +49,7 @@ public enum DTDErrorCode implements IXMLErrorCode {
ElementDeclUnterminated, //
EntityDeclUnterminated, //
EntityNotDeclared, //
EntityExpansionLimitExceeded, //
ExternalIDorPublicIDRequired, //
IDInvalidWithNamespaces, //
IDREFInvalidWithNamespaces, //
Expand Down Expand Up @@ -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;
Expand Down
34 changes: 33 additions & 1 deletion ...e/lemminx/extensions/contentmodel/participants/diagnostics/LSPXMLParserConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}
}
77 changes: 77 additions & 0 deletions ...lemminx/src/test/java/org/eclipse/lemminx/extensions/contentmodel/DTDDiagnosticsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 = "<?xml version=\"1.0\"?>\r\n" + //
"<!DOCTYPE lolz [\r\n" + //
" <!ENTITY lol \"lol\">\r\n" + //
" <!ELEMENT lolz (#PCDATA)>\r\n" + //
" <!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\r\n" + //
" <!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\r\n" + //
" <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\r\n" + //
" <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\r\n" + //
" <!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\r\n" + //
" <!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\r\n" + //
" <!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\r\n" + //
" <!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">\r\n" + //
" <!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">\r\n" + //
"]>\r\n" + //
"<lolz>&lol9;</lolz>";

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 = "<?xml version=\"1.0\"?>\r\n" + //
"<!DOCTYPE lolz [\r\n" + //
" <!ENTITY lol \"lol\">\r\n" + //
" <!ELEMENT lolz (#PCDATA)>\r\n" + //
" <!ENTITY lol1 \"&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;&lol;\">\r\n" + //
" <!ENTITY lol2 \"&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;&lol1;\">\r\n" + //
" <!ENTITY lol3 \"&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;&lol2;\">\r\n" + //
" <!ENTITY lol4 \"&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;&lol3;\">\r\n" + //
" <!ENTITY lol5 \"&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;&lol4;\">\r\n" + //
" <!ENTITY lol6 \"&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;&lol5;\">\r\n" + //
" <!ENTITY lol7 \"&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;&lol6;\">\r\n" + //
" <!ENTITY lol8 \"&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;&lol7;\">\r\n" + //
" <!ENTITY lol9 \"&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;&lol8;\">\r\n" + //
"]>\r\n" + //
"<lolz>&lol9;</lolz>";

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);
}
Expand Down

0 comments on commit 8c4f23f

Please sign in to comment.