Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mitigate Billion Laughs vulnerability #1038

Merged
merged 1 commit into from
May 18, 2021
+113 −1
Merged

Mitigate Billion Laughs vulnerability #1038

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .../src/main/java/org/eclipse/lemminx/extensions/contentmodel/participants/DTDErrorCode.java
View file
Open in desktop
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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is not used

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
Open in desktop
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
Open in desktop
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