Skip to content

Commit

Permalink
Fix for CVE-2017-9805.
Browse files Browse the repository at this point in the history
  • Loading branch information
@joehni
joehni committed Sep 23, 2020
1 parent 573fbaf commit 0fec095
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 29 deletions.
8 changes: 8 additions & 0 deletions xstream-distribution/src/content/changes.html
View file
Expand Up @@ -28,6 +28,14 @@
filter for the appropriate milestone.
</p>

<h1 id="upcoming-1.4.x">Upcoming 1.4.x maintenance release</h1>

<p>Not yet released.</p>

<p class="highlight">This maintenance release addresses the security vulnerability CVE-2017-9805 reported
originally for Struts' XStream Plugin, an arbitrary execution of commands when unmarshalling for XStream instances
with uninitialized security framework.</p>

<h1 id="1.4.13">1.4.13</h1>

<p>Released September 6, 2020.</p>
Expand Down
2 changes: 1 addition & 1 deletion xstream/src/java/com/thoughtworks/xstream/XStream.java
View file
Expand Up @@ -642,7 +642,7 @@ protected void setupSecurity() {
}

addPermission(AnyTypePermission.ANY);
denyTypes(new String[]{"java.beans.EventHandler"});
denyTypes(new String[]{"java.beans.EventHandler", "javax.imageio.ImageIO$ContainsFilter"});
denyTypesByRegExp(new Pattern[]{LAZY_ITERATORS, JAVAX_CRYPTO});
allowTypeHierarchy(Exception.class);
securityInitialized = false;
Expand Down
118 changes: 90 additions & 28 deletions xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java
View file
Expand Up @@ -11,14 +11,14 @@
package com.thoughtworks.acceptance;

import java.beans.EventHandler;
import java.util.Iterator;

import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.XStreamException;
import com.thoughtworks.xstream.converters.ConversionException;
import com.thoughtworks.xstream.converters.reflection.ReflectionConverter;
import com.thoughtworks.xstream.core.JVM;
import com.thoughtworks.xstream.security.AnyTypePermission;
import com.thoughtworks.xstream.security.ForbiddenClassException;
import com.thoughtworks.xstream.security.NoTypePermission;


/**
Expand All @@ -39,15 +39,15 @@ protected void setupSecurity(XStream xstream) {

public void testCannotInjectEventHandler() {
final String xml = ""
+ "<string class='runnable-array'>\n"
+ " <dynamic-proxy>\n"
+ " <interface>java.lang.Runnable</interface>\n"
+ " <handler class='java.beans.EventHandler'>\n"
+ " <target class='com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec'/>\n"
+ " <action>exec</action>\n"
+ " </handler>\n"
+ " </dynamic-proxy>\n"
+ "</string>";
+ "<string class='runnable-array'>\n"
+ " <dynamic-proxy>\n"
+ " <interface>java.lang.Runnable</interface>\n"
+ " <handler class='java.beans.EventHandler'>\n"
+ " <target class='com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec'/>\n"
+ " <action>exec</action>\n"
+ " </handler>\n"
+ " </dynamic-proxy>\n"
+ "</string>";

try {
xstream.fromXML(xml);
Expand Down Expand Up @@ -75,33 +75,96 @@ public void testCannotInjectEventHandlerWithUnconfiguredSecurityFramework() {
xstream.fromXML(xml);
fail("Thrown " + XStreamException.class.getName() + " expected");
} catch (final XStreamException e) {
assertTrue(e.getMessage().indexOf(EventHandler.class.getName())>=0);
assertTrue(e.getMessage().indexOf(EventHandler.class.getName()) >= 0);
}
assertEquals(0, BUFFER.length());
}

public void testExplicitlyConvertEventHandler() {
final String xml = ""
+ "<string class='runnable-array'>\n"
+ " <dynamic-proxy>\n"
+ " <interface>java.lang.Runnable</interface>\n"
+ " <handler class='java.beans.EventHandler'>\n"
+ " <target class='com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec'/>\n"
+ " <action>exec</action>\n"
+ " </handler>\n"
+ " </dynamic-proxy>\n"
+ "</string>";
+ "<string class='runnable-array'>\n"
+ " <dynamic-proxy>\n"
+ " <interface>java.lang.Runnable</interface>\n"
+ " <handler class='java.beans.EventHandler'>\n"
+ " <target class='com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec'/>\n"
+ " <action>exec</action>\n"
+ " </handler>\n"
+ " </dynamic-proxy>\n"
+ "</string>";

xstream.allowTypes(new Class[]{EventHandler.class});
xstream.registerConverter(new ReflectionConverter(xstream.getMapper(), xstream
.getReflectionProvider(), EventHandler.class));

final Runnable[] array = (Runnable[])xstream.fromXML(xml);
assertEquals(0, BUFFER.length());
array[0].run();
assertEquals("Executed!", BUFFER.toString());
}

public void testCannotInjectConvertImageIOContainsFilterWithUnconfiguredSecurityFramework() {
if (JVM.isVersion(7)) {
final String xml = ""
+ "<string class='javax.imageio.spi.FilterIterator'>\n"
+ " <iter class='java.util.ArrayList$Itr'>\n"
+ " <cursor>0</cursor>\n"
+ " <lastRet>1</lastRet>\n"
+ " <expectedModCount>1</expectedModCount>\n"
+ " <outer-class>\n"
+ " <com.thoughtworks.acceptance.SecurityVulnerabilityTest_-Exec/>\n"
+ " </outer-class>\n"
+ " </iter>\n"
+ " <filter class='javax.imageio.ImageIO$ContainsFilter'>\n"
+ " <method>\n"
+ " <class>com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec</class>\n"
+ " <name>exec</name>\n"
+ " <parameter-types/>\n"
+ " </method>\n"
+ " <name>exec</name>\n"
+ " </filter>\n"
+ " <next/>\n"
+ "</string>";

try {
xstream.fromXML(xml);
fail("Thrown " + XStreamException.class.getName() + " expected");
} catch (final XStreamException e) {
assertTrue(e.getMessage().indexOf("javax.imageio.ImageIO$ContainsFilter") >= 0);
}
assertEquals(0, BUFFER.length());
}
}

public void testExplicitlyConvertImageIOContainsFilter() {
if (JVM.isVersion(7)) {
final String xml = ""
+ "<string class='javax.imageio.spi.FilterIterator'>\n"
+ " <iter class='java.util.ArrayList$Itr'>\n"
+ " <cursor>0</cursor>\n"
+ " <lastRet>1</lastRet>\n"
+ " <expectedModCount>1</expectedModCount>\n"
+ " <outer-class>\n"
+ " <com.thoughtworks.acceptance.SecurityVulnerabilityTest_-Exec/>\n"
+ " </outer-class>\n"
+ " </iter>\n"
+ " <filter class='javax.imageio.ImageIO$ContainsFilter'>\n"
+ " <method>\n"
+ " <class>com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec</class>\n"
+ " <name>exec</name>\n"
+ " <parameter-types/>\n"
+ " </method>\n"
+ " <name>exec</name>\n"
+ " </filter>\n"
+ " <next/>\n"
+ "</string>";

xstream.allowTypes(new String[]{"javax.imageio.ImageIO$ContainsFilter"});

final Iterator iterator = (Iterator)xstream.fromXML(xml);
assertEquals(0, BUFFER.length());
iterator.next();
assertEquals("Executed!", BUFFER.toString());
}
}

public static class Exec {

public void exec() {
Expand All @@ -120,7 +183,7 @@ public void testInstanceOfVoid() {

public void testDeniedInstanceOfVoid() {
xstream.addPermission(AnyTypePermission.ANY); // clear out defaults
xstream.denyTypes(new Class[] { void.class, Void.class });
xstream.denyTypes(new Class[]{void.class, Void.class});
try {
xstream.fromXML("<void/>");
fail("Thrown " + ForbiddenClassException.class.getName() + " expected");
Expand All @@ -130,17 +193,16 @@ public void testDeniedInstanceOfVoid() {
}

public void testAllowedInstanceOfVoid() {
xstream.allowTypes(new Class[] { void.class, Void.class });
xstream.allowTypes(new Class[]{void.class, Void.class});
try {
xstream.fromXML("<void/>");
fail("Thrown " + ConversionException.class.getName() + " expected");
} catch (final ConversionException e) {
assertEquals("void", e.get("construction-type"));
}
}

public static class LazyIterator {
}

public static class LazyIterator {}

public void testInstanceOfLazyIterator() {
xstream.alias("lazy-iterator", LazyIterator.class);
Expand Down

0 comments on commit 0fec095

Please sign in to comment.