diff --git a/liquibase-core/src/main/java/liquibase/GlobalConfiguration.java b/liquibase-core/src/main/java/liquibase/GlobalConfiguration.java index c08bf90d587..a948b8779ec 100644 --- a/liquibase-core/src/main/java/liquibase/GlobalConfiguration.java +++ b/liquibase-core/src/main/java/liquibase/GlobalConfiguration.java @@ -33,6 +33,7 @@ public class GlobalConfiguration implements AutoloadedConfigurations { public static final ConfigurationDefinition HEADLESS; public static final ConfigurationDefinition STRICT; public static final ConfigurationDefinition DDL_LOCK_TIMEOUT; + public static final ConfigurationDefinition SECURE_PARSING; static { ConfigurationDefinition.Builder builder = new ConfigurationDefinition.Builder("liquibase"); @@ -175,5 +176,10 @@ public class GlobalConfiguration implements AutoloadedConfigurations { .addAliasKey("liquibase.ddl_lock_timeout") .setDescription("The DDL_LOCK_TIMEOUT parameter indicates the number of seconds a DDL command should wait for the locks to become available before throwing the resource busy error message. This applies only to Oracle databases.") .build(); + + SECURE_PARSING = builder.define("secureParsing", Boolean.class) + .setDescription("If true, remove functionality from file parsers which could be used insecurely. Examples include (but not limited to) disabling remote XML entity support.") + .setDefaultValue(true) + .build(); } } diff --git a/liquibase-core/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java b/liquibase-core/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java index d8eb80e4604..ef3c8bb5515 100644 --- a/liquibase-core/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java +++ b/liquibase-core/src/main/java/liquibase/parser/core/xml/XMLChangeLogSAXParser.java @@ -1,5 +1,6 @@ package liquibase.parser.core.xml; +import liquibase.GlobalConfiguration; import liquibase.Scope; import liquibase.changelog.ChangeLogParameters; import liquibase.exception.ChangeLogParseException; @@ -9,6 +10,7 @@ import liquibase.util.FileUtil; import org.xml.sax.*; +import javax.xml.XMLConstants; import javax.xml.parsers.SAXParser; import javax.xml.parsers.SAXParserFactory; import java.io.IOException; @@ -25,6 +27,13 @@ public XMLChangeLogSAXParser() { saxParserFactory = SAXParserFactory.newInstance(); saxParserFactory.setValidating(true); saxParserFactory.setNamespaceAware(true); + if (GlobalConfiguration.SECURE_PARSING.getCurrentValue()) { + try { + saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); + } catch (Throwable e) { + Scope.getCurrentScope().getLog(getClass()).fine("Cannot enable FEATURE_SECURE_PROCESSING: " + e.getMessage(), e); + } + } } @Override @@ -49,6 +58,13 @@ protected SAXParserFactory getSaxParserFactory() { protected ParsedNode parseToNode(String physicalChangeLogLocation, ChangeLogParameters changeLogParameters, ResourceAccessor resourceAccessor) throws ChangeLogParseException { try (InputStream inputStream = resourceAccessor.openStream(null, physicalChangeLogLocation)) { SAXParser parser = saxParserFactory.newSAXParser(); + if (GlobalConfiguration.SECURE_PARSING.getCurrentValue()) { + try { + parser.setProperty(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "http,https"); //need to allow external schemas on http/https to support the liquibase.org xsd files + } catch (SAXException e) { + Scope.getCurrentScope().getLog(getClass()).fine("Cannot enable ACCESS_EXTERNAL_SCHEMA: " + e.getMessage(), e); + } + } trySetSchemaLanguageProperty(parser); XMLReader xmlReader = parser.getXMLReader(); diff --git a/liquibase-core/src/test/groovy/liquibase/parser/core/xml/XMLChangeLogSAXParserTest.groovy b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/XMLChangeLogSAXParserTest.groovy new file mode 100644 index 00000000000..654db821c53 --- /dev/null +++ b/liquibase-core/src/test/groovy/liquibase/parser/core/xml/XMLChangeLogSAXParserTest.groovy @@ -0,0 +1,103 @@ +package liquibase.parser.core.xml + +import liquibase.Contexts +import liquibase.GlobalConfiguration +import liquibase.LabelExpression +import liquibase.RuntimeEnvironment +import liquibase.Scope +import liquibase.changelog.ChangeLogIterator +import liquibase.changelog.ChangeLogParameters +import liquibase.changelog.ChangeSet +import liquibase.changelog.DatabaseChangeLog +import liquibase.changelog.filter.ChangeSetFilterResult +import liquibase.changelog.visitor.ChangeSetVisitor +import liquibase.database.Database +import liquibase.database.core.MockDatabase +import liquibase.exception.ChangeLogParseException +import liquibase.exception.LiquibaseException +import liquibase.sdk.resource.MockResourceAccessor +import liquibase.test.JUnitResourceAccessor +import spock.lang.Specification + +class XMLChangeLogSAXParserTest extends Specification { + + def INSECURE_XML = """ + + ]> + + + + + &insecure; + + + +""" + + def testIgnoreDuplicateChangeSets() throws ChangeLogParseException, Exception { + when: + def xmlParser = new XMLChangeLogSAXParser() + def changeLog = xmlParser.parse("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/master.changelog.xml", + new ChangeLogParameters(), new JUnitResourceAccessor()) + + final List changeSets = new ArrayList() + + new ChangeLogIterator(changeLog).run(new ChangeSetVisitor() { + @Override + ChangeSetVisitor.Direction getDirection() { + return ChangeSetVisitor.Direction.FORWARD + } + + @Override + void visit(ChangeSet changeSet, DatabaseChangeLog databaseChangeLog, Database database, Set filterResults) throws LiquibaseException { + changeSets.add(changeSet) + } + }, new RuntimeEnvironment(new MockDatabase(), new Contexts(), new LabelExpression())) + + + then: + changeSets.size() == 8 + changeSets.get(0).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser" + changeSets.get(1).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser" + changeSets.get(1).getContexts().getContexts().size() == 1 + changeSets.get(2).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser" + changeSets.get(2).getLabels().getLabels().size() == 1 + changeSets.get(3).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser" + changeSets.get(3).getLabels().getLabels().size() == 2 + changeSets.get(4).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser" + changeSets.get(4).getDbmsSet().size() == 1 + changeSets.get(5).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog1.xml::1::testuser" + changeSets.get(6).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog3.xml::1::testuser" + changeSets.get(7).toString() == "liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog2.xml::1::testuser" + } + + def "uses liquibase.secureParsing by default"() { + when: + def resourceAccessor = new MockResourceAccessor(["com/example/insecure.xml": INSECURE_XML]) + + new XMLChangeLogSAXParser().parse("com/example/insecure.xml", new ChangeLogParameters(), resourceAccessor) + + then: + def e = thrown(ChangeLogParseException) + e.message.contains("access is not allowed due to restriction set by the accessExternalDTD property") + } + + def "allows liquibase.secureParsing=false to disable secure parsing"() { + when: + def resourceAccessor = new MockResourceAccessor(["com/example/insecure.xml": INSECURE_XML]) + + Scope.child(GlobalConfiguration.SECURE_PARSING.key, "false", { -> + new XMLChangeLogSAXParser().parse("com/example/insecure.xml", new ChangeLogParameters(), resourceAccessor) + }) + + + then: + def e = thrown(ChangeLogParseException) + e.message.contains("Error Reading Changelog File: invalid.txt") + } + +} diff --git a/liquibase-core/src/test/java/liquibase/parser/core/xml/XMLChangeLogSAXParserTest.java b/liquibase-core/src/test/java/liquibase/parser/core/xml/XMLChangeLogSAXParserTest.java deleted file mode 100644 index ff58dcb85f6..00000000000 --- a/liquibase-core/src/test/java/liquibase/parser/core/xml/XMLChangeLogSAXParserTest.java +++ /dev/null @@ -1,76 +0,0 @@ -package liquibase.parser.core.xml; - -import liquibase.Contexts; -import liquibase.LabelExpression; -import liquibase.RuntimeEnvironment; -import liquibase.changelog.ChangeLogIterator; -import liquibase.changelog.ChangeLogParameters; -import liquibase.changelog.ChangeSet; -import liquibase.changelog.DatabaseChangeLog; -import liquibase.changelog.filter.ChangeSetFilterResult; -import liquibase.changelog.visitor.ChangeSetVisitor; -import liquibase.database.Database; -import liquibase.database.core.MockDatabase; -import liquibase.exception.ChangeLogParseException; -import liquibase.exception.LiquibaseException; -import liquibase.test.JUnitResourceAccessor; -import org.junit.Assert; -import org.junit.Test; - -import java.util.ArrayList; -import java.util.List; -import java.util.Set; - -public class XMLChangeLogSAXParserTest { - - @Test - public void testIgnoreDuplicateChangeSets() throws ChangeLogParseException, Exception { - XMLChangeLogSAXParser xmlParser = new XMLChangeLogSAXParser(); - DatabaseChangeLog changeLog = xmlParser.parse("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/master.changelog.xml", - new ChangeLogParameters(), new JUnitResourceAccessor()); - - final List changeSets = new ArrayList(); - - new ChangeLogIterator(changeLog).run(new ChangeSetVisitor() { - @Override - public Direction getDirection() { - return Direction.FORWARD; - } - - @Override - public void visit(ChangeSet changeSet, DatabaseChangeLog databaseChangeLog, Database database, Set filterResults) throws LiquibaseException { - changeSets.add(changeSet); - } - }, new RuntimeEnvironment(new MockDatabase(), new Contexts(), new LabelExpression())); - - - Assert.assertEquals(8, changeSets.size()); - - Assert.assertEquals("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser", - changeSets.get(0).toString()); - - Assert.assertEquals("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser", - changeSets.get(1).toString()); - Assert.assertEquals(1, changeSets.get(1).getContexts().getContexts().size()); - - Assert.assertEquals("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser", - changeSets.get(2).toString()); - Assert.assertEquals(1, changeSets.get(2).getLabels().getLabels().size()); - - Assert.assertEquals("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser", - changeSets.get(3).toString()); - Assert.assertEquals(2, changeSets.get(3).getLabels().getLabels().size()); - - Assert.assertEquals("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog4.xml::1::testuser", - changeSets.get(4).toString()); - Assert.assertEquals(1, changeSets.get(4).getDbmsSet().size()); - - Assert.assertEquals("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog1.xml::1::testuser", - changeSets.get(5).toString()); - Assert.assertEquals("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog3.xml::1::testuser", - changeSets.get(6).toString()); - Assert.assertEquals("liquibase/parser/core/xml/ignoreDuplicatedChangeLogs/included.changelog2.xml::1::testuser", - changeSets.get(7).toString()); - } - -} \ No newline at end of file