From e8e88621ba1c85ac3b8620337dd672e0c0c3a846 Mon Sep 17 00:00:00 2001 From: joehni Date: Mon, 27 Dec 2021 01:24:08 +0100 Subject: [PATCH] Describe and fix CVE-2021-43859. --- .../src/content/CVE-2021-43859.html | 199 ++++++++++++++++++ .../src/content/security.html | 40 +++- xstream-distribution/src/content/website.xml | 1 + .../com/thoughtworks/xstream/XStream.java | 42 +++- .../collections/CollectionConverter.java | 6 +- .../converters/collections/MapConverter.java | 6 +- .../extended/NamedMapConverter.java | 5 +- .../xstream/core/SecurityUtils.java | 56 +++++ .../xstream/core/TreeUnmarshaller.java | 3 + .../security/AbstractSecurityException.java | 29 +++ .../security/ForbiddenClassException.java | 6 +- .../security/InputManipulationException.java | 27 +++ .../acceptance/SecurityVulnerabilityTest.java | 155 +++++++++++++- 14 files changed, 568 insertions(+), 21 deletions(-) create mode 100644 xstream-distribution/src/content/CVE-2021-43859.html create mode 100644 xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java create mode 100644 xstream/src/java/com/thoughtworks/xstream/security/AbstractSecurityException.java create mode 100644 xstream/src/java/com/thoughtworks/xstream/security/InputManipulationException.java diff --git a/xstream-distribution/src/content/CVE-2021-43859.html b/xstream-distribution/src/content/CVE-2021-43859.html new file mode 100644 index 00000000..531298d3 --- /dev/null +++ b/xstream-distribution/src/content/CVE-2021-43859.html @@ -0,0 +1,199 @@ + + + + CVE-2021-43859 + + + +

Vulnerability

+ +

CVE-2021-43859: XStream can cause a Denial of Service by injecting highly recursive collections or maps.

+ +

Affected Versions

+ +

All versions until and including version 1.4.18 are affected.

+ +

Description

+ +

The processed stream at unmarshalling time contains type information to recreate the formerly written objects. + XStream creates therefore new instances based on these type information. An attacker can manipulate the processed + input stream and replace or inject objects, that result in exponential recursively hashcode calculation, causing a denial + of service.

+ +

Steps to Reproduce

+ +

The attack uses the hashcode implementation of collection types in the Java runtime. Following types are affected with + lastest Java versions available in December 2021:

+ +

Create a simple HashSet and use XStream to marshal it to XML. Replace the XML with following snippet, increase the + depth of the structure and unmarshal it with XStream:

+
<set>
+  <set>
+    <string>a</string>
+    <set>
+      <string>a</string>
+      <set>
+        <string>a</string>
+      </set>
+      <set>
+        <string>b</string>
+      </set>
+    </set>
+    <set>
+      <set reference="../../set/set"/>
+      <string>b</string>
+      <set reference="../../set/set[2]"/>
+    </set>
+  </set>
+  <set>
+    <set reference="../../set/set"/>
+    <string>b</string>
+    <set reference="../../set/set[2]"/>
+  </set>
+</set>
+
+
XStream xstream = new XStream();
+xstream.fromXML(xml);
+
+

Create a simple HashMap and use XStream to marshal it to XML. Replace the XML with following snippet, increase the + depth of the structure and unmarshal it with XStream:

+
<map>
+  <entry>
+    <map>
+      <entry>
+        <string>a</string>
+        <string>b</string>
+      </entry>
+      <entry>
+        <map>
+          <entry>
+            <string>a</string>
+            <string>b</string>
+          </entry>
+          <entry>
+            <map>
+              <entry>
+                <string>a</string>
+                <string>b</string>
+              </entry>
+            </map>
+            <map>
+              <entry>
+                <string>c</string>
+                <string>d</string>
+              </entry>
+            </map>
+          </entry>
+          <entry>
+            <map reference="../../entry[2]/map[2]"/>
+            <map reference="../../entry[2]/map"/>
+          </entry>
+        </map>
+        <map>
+          <entry>
+            <string>c</string>
+            <string>d</string>
+          </entry>
+          <entry>
+            <map reference="../../../entry[2]/map"/>
+            <map reference="../../../entry[2]/map[2]"/>
+          </entry>
+          <entry>
+            <map reference="../../../entry[2]/map[2]"/>
+            <map reference="../../../entry[2]/map"/>
+          </entry>
+        </map>
+      </entry>
+      <entry>
+        <map reference="../../entry[2]/map[2]"/>
+        <map reference="../../entry[2]/map"/>
+      </entry>
+    </map>
+    <map>
+      <entry>
+        <string>c</string>
+        <string>d</string>
+      </entry>
+      <entry>
+        <map reference="../../../entry[2]/map"/>
+        <map reference="../../../entry[2]/map[2]"/>
+      </entry>
+      <entry>
+        <map reference="../../../entry[2]/map[2]"/>
+        <map reference="../../../entry[2]/map"/>
+      </entry>
+    </map>
+  </entry>
+  <entry>
+    <map reference="../../entry[2]/map[2]"/>
+    <map reference="../../entry[2]/map"/>
+  </entry>
+</map>
+
+
XStream xstream = new XStream();
+xstream.fromXML(xml);
+
+ +

As soon as the XML is unmarshalled, the hash codes of the elements are calculated and the calculation time increases + exponentially due to the highly recursive structure.

+ +

Note, this example uses XML, but the attack can be performed for any supported format, that supports references, i.e. + JSON is not affected.

+ +

Impact

+ +

The vulnerability may allow a remote attacker to allocate 100% CPU time on the target system depending on CPU + type or parallel execution of such a payload resulting in a denial of service only by manipulating the processed + input stream.

+ +

Workarounds

+ +

If your object graph does not use referenced elements at all, you may simply set the NO_REFERENCE mode:

+ +
XStream xstream = new XStream();
+xstream.setMode(XStream.NO_REFERENCES);
+
+ +

If your object graph contains neither a Hashtable, HashMap nor a HashSet (or one of the linked variants of it) then you + can use the security framework to deny the usage of these types:

+ +
XStream xstream = new XStream();
+xstream.denyTypes(new Class[]{
+	java.util.HashMap.class, java.util.HashSet.class, java.util.Hashtable.class, java.util.LinkedHashMap.class, java.util.LinkedHashSet.class
+});
+
+ +

Unfortunately these types are very common. If you only use HashMap or HashSet and your XML refers these only as default + map or set, you may additionally change the default implementation of java.util.Map and java.util.Set at unmarshalling time:

+ +
xstream.addDefaultImplementation(java.util.TreeMap.class, java.util.Map.class);
+xstream.addDefaultImplementation(java.util.TreeSet.class, java.util.Set.class);
+
+ +

However, this implies that your application does not care about the implementation of the map and all elements are comparable.

+ +

Credits

+ +

r00t4dm at Cloud-Penetrating Arrow Lab found and reported the issue to XStream and provided the required information to + reproduce it.

+ + + diff --git a/xstream-distribution/src/content/security.html b/xstream-distribution/src/content/security.html index f0e0177c..fe0a8217 100644 --- a/xstream-distribution/src/content/security.html +++ b/xstream-distribution/src/content/security.html @@ -30,13 +30,13 @@ context of the server running the XStream process or cause a denial of service by crashing the application or manage to enter an endless loop consuming 100% of CPU cycles.

-

Note: XStream supports other data formats than XML, e.g. JSON. Those formats can be used for - the same attacks.

+

Note: XStream supports other data formats than XML, e.g. JSON. Those formats can usually be used + for the same attacks.

-

Note, that the XML data can be manipulated on different levels. For example, manipulating values on existing - objects (such as a price value), accessing private data, or breaking the format and causing the XML parser to fail. - The latter case will raise an exception, but the former case must be handled by validity checks in any application - which processes user-supplied XML.

+

The XML data can be manipulated on different levels. For example, manipulating values on existing objects (such + as a price value), accessing private data, or breaking the format and causing the XML parser to fail. The latter + case will raise an exception, but the former case must be handled by validity checks in any application which + processes user-supplied XML.

Documented Vulnerabilities

@@ -49,6 +49,14 @@

Documented Vulnerabilities

CVE Description + + Version 1.4.18 + + + + CVE-2021-43859 + XStream can cause a Denial of Service by injecting highly recursive collections or maps. + Version 1.4.17 @@ -258,6 +266,16 @@

Implicit Security

because no-one can assure, that no other vulnerability is found. A better approach is the usage of a whitelist i.e. the allowed class types are setup explicitly. This is the default for XStream 1.4.18 (see below).

+

XStream supports references to objects already occuring on the object graph in an earlier location. This allows + an attacker to create a highly recursive object structure. Some collections or maps calculate the position of a + member based on the data of the member itself. This is true for sorting collections or maps, but also for + collections or maps based on the hash code of the individual members. The calculation time for the member's + position can increase exponentially depending on the recursive depth of the structure and cause therefore a Denial + of Service. Therefore XStream measures the time consumed to add an element to a collection or map since version + 1.4.19. Normally this operation is performed in a view milliseconds, but if adding elements take longer than a + second, then the time is accumulated and an exception is thrown if it exceeds a definable limit (20 seconds by + default).

+

Explicit Security

    

Starting with XStream 1.4.7, it is possible to define permissions for types, to check @@ -285,6 +303,16 @@

Explicit Security

Apart from value manipulations, this implementation still allows the injection of allowed objects at wrong locations, e.g. inserting an integer into a list of strings.

+

To avoid an attack based on the position of an element in a collection or map, you should also use XStream's + default converters for 3rd party or own implementations of collections or maps. Own custom converters of such + types should measure the time to add an element at deserialization time using the following sequence in the + implementation of the unmarshal method:

+
// unmarshal element of collection 
+long now = System.currentTimeMillis();
+// add element here, e.g. list.add(element);
+SecurityUtils.checkForCollectionDoSAttack(context, now);
+

+

XML Validation

XML itself supports input validation using a schema and a validating parser. With XStream, you can use e.g. a diff --git a/xstream-distribution/src/content/website.xml b/xstream-distribution/src/content/website.xml index 157baeb9..ad85d03d 100644 --- a/xstream-distribution/src/content/website.xml +++ b/xstream-distribution/src/content/website.xml @@ -89,6 +89,7 @@ CVE-2021-39152.html CVE-2021-39153.html CVE-2021-39154.html + CVE-2021-43859.html CVE-2020-26217.html CVE-2020-26258.html CVE-2020-26259.html diff --git a/xstream/src/java/com/thoughtworks/xstream/XStream.java b/xstream/src/java/com/thoughtworks/xstream/XStream.java index 7d90dc7f..9787059d 100644 --- a/xstream/src/java/com/thoughtworks/xstream/XStream.java +++ b/xstream/src/java/com/thoughtworks/xstream/XStream.java @@ -151,6 +151,7 @@ import com.thoughtworks.xstream.mapper.XStream11XmlFriendlyMapper; import com.thoughtworks.xstream.security.AnyTypePermission; import com.thoughtworks.xstream.security.ArrayTypePermission; +import com.thoughtworks.xstream.security.InputManipulationException; import com.thoughtworks.xstream.security.ExplicitTypePermission; import com.thoughtworks.xstream.security.InterfaceTypePermission; import com.thoughtworks.xstream.security.NoPermission; @@ -295,6 +296,8 @@ // CAUTION: The sequence of the fields is intentional for an optimal XML output of a // self-serialization! + private int collectionUpdateLimit = 20; + private ReflectionProvider reflectionProvider; private HierarchicalStreamDriver hierarchicalStreamDriver; private ClassLoaderReference classLoaderReference; @@ -329,6 +332,9 @@ public static final int PRIORITY_LOW = -10; public static final int PRIORITY_VERY_LOW = -20; + public static final String COLLECTION_UPDATE_LIMIT = "XStreamCollectionUpdateLimit"; + public static final String COLLECTION_UPDATE_SECONDS = "XStreamCollectionUpdateSeconds"; + private static final String ANNOTATION_MAPPER_TYPE = "com.thoughtworks.xstream.mapper.AnnotationMapper"; private static final Pattern IGNORE_ALL = Pattern.compile(".*"); @@ -1182,6 +1188,23 @@ public void setMarshallingStrategy(MarshallingStrategy marshallingStrategy) { this.marshallingStrategy = marshallingStrategy; } + /** + * Set time limit for adding elements to collections or maps. + * + * Manipulated content may be used to create recursive hash code calculations or sort operations. An + * {@link InputManipulationException} is thrown, it the summed up time to add elements to collections or maps + * exceeds the provided limit. + * + * Note, that the time to add an individual element is calculated in seconds, not milliseconds. However, attacks + * typically use objects with exponential growing calculation times. + * + * @param maxSeconds limit in seconds or 0 to disable check + * @since upcoming + */ + public void setCollectionUpdateLimit(int maxSeconds) { + collectionUpdateLimit = maxSeconds; + } + /** * Serialize an object to a pretty-printed XML String. * @@ -1388,6 +1411,13 @@ public Object unmarshal(HierarchicalStreamReader reader, Object root) { */ public Object unmarshal(HierarchicalStreamReader reader, Object root, DataHolder dataHolder) { try { + if (collectionUpdateLimit >= 0) { + if (dataHolder == null) { + dataHolder = new MapBackedDataHolder(); + } + dataHolder.put(COLLECTION_UPDATE_LIMIT, new Integer(collectionUpdateLimit)); + dataHolder.put(COLLECTION_UPDATE_SECONDS, new Integer(0)); + } return marshallingStrategy.unmarshal(root, reader, dataHolder, converterLookup, mapper); } catch (ConversionException e) { Package pkg = getClass().getPackage(); @@ -2053,15 +2083,23 @@ public ObjectInputStream createObjectInputStream(final HierarchicalStreamReader * @see #createObjectInputStream(com.thoughtworks.xstream.io.HierarchicalStreamReader) * @since 1.4.10 */ - public ObjectInputStream createObjectInputStream(final HierarchicalStreamReader reader, final DataHolder dataHolder) + public ObjectInputStream createObjectInputStream(final HierarchicalStreamReader reader, DataHolder dataHolder) throws IOException { + if (collectionUpdateLimit >= 0) { + if (dataHolder == null) { + dataHolder = new MapBackedDataHolder(); + } + dataHolder.put(COLLECTION_UPDATE_LIMIT, new Integer(collectionUpdateLimit)); + dataHolder.put(COLLECTION_UPDATE_SECONDS, new Integer(0)); + } + final DataHolder dh = dataHolder; return new CustomObjectInputStream(new CustomObjectInputStream.StreamCallback() { public Object readFromStream() throws EOFException { if (!reader.hasMoreChildren()) { throw new EOFException(); } reader.moveDown(); - final Object result = unmarshal(reader, null, dataHolder); + final Object result = unmarshal(reader, null, dh); reader.moveUp(); return result; } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/CollectionConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/CollectionConverter.java index 94474193..f3606f1d 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/CollectionConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/CollectionConverter.java @@ -1,6 +1,6 @@ /* * Copyright (C) 2003, 2004, 2005 Joe Walnes. - * Copyright (C) 2006, 2007, 2010, 2011, 2013, 2018 XStream Committers. + * Copyright (C) 2006, 2007, 2010, 2011, 2013, 2018, 2021 XStream Committers. * All rights reserved. * * The software in this package is published under the terms of the BSD @@ -13,6 +13,7 @@ import com.thoughtworks.xstream.converters.MarshallingContext; import com.thoughtworks.xstream.converters.UnmarshallingContext; +import com.thoughtworks.xstream.core.SecurityUtils; import com.thoughtworks.xstream.io.HierarchicalStreamReader; import com.thoughtworks.xstream.io.HierarchicalStreamWriter; import com.thoughtworks.xstream.mapper.Mapper; @@ -96,7 +97,10 @@ protected void populateCollection(HierarchicalStreamReader reader, Unmarshalling protected void addCurrentElementToCollection(HierarchicalStreamReader reader, UnmarshallingContext context, Collection collection, Collection target) { final Object item = readItem(reader, context, collection); // call readBareItem when deprecated method is removed + + long now = System.currentTimeMillis(); target.add(item); + SecurityUtils.checkForCollectionDoSAttack(context, now); } protected Object createCollection(Class type) { diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/MapConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/MapConverter.java index af007f95..f93cec8e 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/MapConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/MapConverter.java @@ -1,6 +1,6 @@ /* * Copyright (C) 2003, 2004, 2005 Joe Walnes. - * Copyright (C) 2006, 2007, 2008, 2010, 2011, 2012, 2013, 2018 XStream Committers. + * Copyright (C) 2006, 2007, 2008, 2010, 2011, 2012, 2013, 2018, 2021 XStream Committers. * All rights reserved. * * The software in this package is published under the terms of the BSD @@ -13,6 +13,7 @@ import com.thoughtworks.xstream.converters.MarshallingContext; import com.thoughtworks.xstream.converters.UnmarshallingContext; +import com.thoughtworks.xstream.core.SecurityUtils; import com.thoughtworks.xstream.io.ExtendedHierarchicalStreamWriterHelper; import com.thoughtworks.xstream.io.HierarchicalStreamReader; import com.thoughtworks.xstream.io.HierarchicalStreamWriter; @@ -104,7 +105,10 @@ protected void putCurrentEntryIntoMap(HierarchicalStreamReader reader, Unmarshal Map map, Map target) { final Object key = readCompleteItem(reader, context, map); final Object value = readCompleteItem(reader, context, map); + + long now = System.currentTimeMillis(); target.put(key, value); + SecurityUtils.checkForCollectionDoSAttack(context, now); } protected Object createCollection(Class type) { diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/extended/NamedMapConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/extended/NamedMapConverter.java index 4c5ec9cf..59021489 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/extended/NamedMapConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/extended/NamedMapConverter.java @@ -1,5 +1,5 @@ /* - * Copyright (C) 2013, 2016, 2018 XStream Committers. + * Copyright (C) 2013, 2016, 2018, 2021 XStream Committers. * All rights reserved. * * The software in this package is published under the terms of the BSD @@ -21,6 +21,7 @@ import com.thoughtworks.xstream.converters.UnmarshallingContext; import com.thoughtworks.xstream.converters.collections.MapConverter; import com.thoughtworks.xstream.core.JVM; +import com.thoughtworks.xstream.core.SecurityUtils; import com.thoughtworks.xstream.core.util.HierarchicalStreams; import com.thoughtworks.xstream.io.ExtendedHierarchicalStreamWriterHelper; import com.thoughtworks.xstream.io.HierarchicalStreamReader; @@ -339,7 +340,9 @@ protected void populateMap(HierarchicalStreamReader reader, UnmarshallingContext value = valueConverter.fromString(reader.getValue()); } + long now = System.currentTimeMillis(); target.put(key, value); + SecurityUtils.checkForCollectionDoSAttack(context, now); if (entryName != null) { reader.moveUp(); diff --git a/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java b/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java new file mode 100644 index 00000000..0eedd523 --- /dev/null +++ b/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2021 XStream Committers. + * All rights reserved. + * + * The software in this package is published under the terms of the BSD + * style license a copy of which has been included with this distribution in + * the LICENSE.txt file. + * + * Created on 21. September 2021 by Joerg Schaible + */ +package com.thoughtworks.xstream.core; + +import com.thoughtworks.xstream.XStream; +import com.thoughtworks.xstream.converters.ConversionException; +import com.thoughtworks.xstream.converters.UnmarshallingContext; +import com.thoughtworks.xstream.security.InputManipulationException; + + +/** + * Utility functions for security issues. + * + * @author Jörg Schaible + * @since upcoming + */ +public class SecurityUtils { + + /** + * Check the consumed time adding elements to collections or maps. + * + * Every custom converter should call this method after an unmarshalled element has been added to a collection or + * map. In case of an attack the operation will take too long, because the calculation of the hash code or the + * comparison of the elements in the collection operate on recursive structures. + * + * @param context the unmarshalling context + * @param start the timestamp just before the element was added to the collection or map + * @since upcoming + */ + public static void checkForCollectionDoSAttack(final UnmarshallingContext context, final long start) { + final int diff = (int)((System.currentTimeMillis() - start) / 1000); + if (diff > 0) { + final Integer secondsUsed = (Integer)context.get(XStream.COLLECTION_UPDATE_SECONDS); + if (secondsUsed != null) { + final Integer limit = (Integer)context.get(XStream.COLLECTION_UPDATE_LIMIT); + if (limit == null) { + throw new ConversionException("Missing limit for updating collections."); + } + final int seconds = secondsUsed.intValue() + diff; + if (seconds > limit.intValue()) { + throw new InputManipulationException( + "Denial of Service attack assumed. Adding elements to collections or maps exceeds " + limit.intValue() + " seconds."); + } + context.put(XStream.COLLECTION_UPDATE_SECONDS, new Integer(seconds)); + } + } + } +} diff --git a/xstream/src/java/com/thoughtworks/xstream/core/TreeUnmarshaller.java b/xstream/src/java/com/thoughtworks/xstream/core/TreeUnmarshaller.java index a17363f4..be1ef0d7 100644 --- a/xstream/src/java/com/thoughtworks/xstream/core/TreeUnmarshaller.java +++ b/xstream/src/java/com/thoughtworks/xstream/core/TreeUnmarshaller.java @@ -26,6 +26,7 @@ import com.thoughtworks.xstream.core.util.PrioritizedList; import com.thoughtworks.xstream.io.HierarchicalStreamReader; import com.thoughtworks.xstream.mapper.Mapper; +import com.thoughtworks.xstream.security.AbstractSecurityException; public class TreeUnmarshaller implements UnmarshallingContext { @@ -74,6 +75,8 @@ protected Object convert(Object parent, Class type, Converter converter) { } catch (final ConversionException conversionException) { addInformationTo(conversionException, type, converter, parent); throw conversionException; + } catch (AbstractSecurityException e) { + throw e; } catch (RuntimeException e) { ConversionException conversionException = new ConversionException(e); addInformationTo(conversionException, type, converter, parent); diff --git a/xstream/src/java/com/thoughtworks/xstream/security/AbstractSecurityException.java b/xstream/src/java/com/thoughtworks/xstream/security/AbstractSecurityException.java new file mode 100644 index 00000000..3ca6309c --- /dev/null +++ b/xstream/src/java/com/thoughtworks/xstream/security/AbstractSecurityException.java @@ -0,0 +1,29 @@ +/* + * Copyright (C) 2021 XStream Committers. + * All rights reserved. + * + * Created on 21. September 2021 by Joerg Schaible + */ +package com.thoughtworks.xstream.security; + +import com.thoughtworks.xstream.XStreamException; + + +/** + * General base class for a Security Exception in XStream. + * + * @author Jörg Schaible + * @since upcoming + */ +public abstract class AbstractSecurityException extends XStreamException { + private static final long serialVersionUID = 20210921L; + + /** + * Constructs a SecurityException. + * @param message the exception message + * @since upcoming + */ + public AbstractSecurityException(final String message) { + super(message); + } +} diff --git a/xstream/src/java/com/thoughtworks/xstream/security/ForbiddenClassException.java b/xstream/src/java/com/thoughtworks/xstream/security/ForbiddenClassException.java index 017fc301..2eded6cf 100644 --- a/xstream/src/java/com/thoughtworks/xstream/security/ForbiddenClassException.java +++ b/xstream/src/java/com/thoughtworks/xstream/security/ForbiddenClassException.java @@ -1,20 +1,18 @@ /* - * Copyright (C) 2014 XStream Committers. + * Copyright (C) 2014, 2021 XStream Committers. * All rights reserved. * * Created on 08. January 2014 by Joerg Schaible */ package com.thoughtworks.xstream.security; -import com.thoughtworks.xstream.XStreamException; - /** * Exception thrown for a forbidden class. * * @author Jörg Schaible * @since 1.4.7 */ -public class ForbiddenClassException extends XStreamException { +public class ForbiddenClassException extends AbstractSecurityException { /** * Construct a ForbiddenClassException. diff --git a/xstream/src/java/com/thoughtworks/xstream/security/InputManipulationException.java b/xstream/src/java/com/thoughtworks/xstream/security/InputManipulationException.java new file mode 100644 index 00000000..2d87f660 --- /dev/null +++ b/xstream/src/java/com/thoughtworks/xstream/security/InputManipulationException.java @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2021 XStream Committers. + * All rights reserved. + * + * Created on 21. September 2021 by Joerg Schaible + */ +package com.thoughtworks.xstream.security; + + +/** + * Class for a Security Exception assuming input manipulation in XStream. + * + * @author Jörg Schaible + * @since upcoming + */ +public class InputManipulationException extends AbstractSecurityException { + private static final long serialVersionUID = 20210921L; + + /** + * Constructs a SecurityException. + * @param message the exception message + * @since upcoming + */ + public InputManipulationException(final String message) { + super(message); + } +} diff --git a/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java b/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java index 09b96a8d..167939d7 100644 --- a/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java +++ b/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java @@ -17,13 +17,20 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Hashtable; import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Set; -import com.thoughtworks.xstream.XStreamException; import com.thoughtworks.xstream.converters.ConversionException; import com.thoughtworks.xstream.core.JVM; import com.thoughtworks.xstream.security.AnyTypePermission; import com.thoughtworks.xstream.security.ForbiddenClassException; +import com.thoughtworks.xstream.security.InputManipulationException; import com.thoughtworks.xstream.security.ProxyTypePermission; @@ -56,9 +63,9 @@ public void testCannotInjectEventHandler() { try { xstream.fromXML(xml); - fail("Thrown " + XStreamException.class.getName() + " expected"); - } catch (final XStreamException e) { - assertTrue(e.getMessage().indexOf(EventHandler.class.getName()) > 0); + fail("Thrown " + ForbiddenClassException.class.getName() + " expected"); + } catch (final ForbiddenClassException e) { + // OK } assertEquals(0, BUFFER.length()); } @@ -126,7 +133,7 @@ public void exec() { public void testInstanceOfVoid() { try { xstream.fromXML(""); - fail("Thrown " + ConversionException.class.getName() + " expected"); + fail("Thrown " + ForbiddenClassException.class.getName() + " expected"); } catch (final ForbiddenClassException e) { // OK } @@ -163,7 +170,7 @@ public void testCannotUseJaxwsInputStreamToDeleteFile() { xstream.aliasType("is", InputStream.class); try { xstream.fromXML(xml); - fail("Thrown " + ConversionException.class.getName() + " expected"); + fail("Thrown " + ForbiddenClassException.class.getName() + " expected"); } catch (final ForbiddenClassException e) { // OK } @@ -261,4 +268,140 @@ public void testExplicitlyUnmarshalEndlessByteArryInputStream() { assertEquals("ArrayIndexOutOfBoundsException expected reading invalid stream", 5, i); } } + + public void testDoSAttackWithHashSet() { + final Set set = new HashSet(); + Set s1 = set; + Set s2 = new HashSet(); + for (int i = 0; i < 30; i++) { + final Set t1 = new HashSet(); + final Set t2 = new HashSet(); + t1.add("a"); + t2.add("b"); + s1.add(t1); + s1.add(t2); + s2.add(t2); + s2.add(t1); + s1 = t1; + s2 = t2; + } + + xstream.setCollectionUpdateLimit(5); + final String xml = xstream.toXML(set); + try { + + xstream.fromXML(xml); + fail("Thrown " + InputManipulationException.class.getName() + " expected"); + } catch (final InputManipulationException e) { + assertTrue("Limit expected in message", e.getMessage().contains("exceeds 5 seconds")); + } + } + + public void testDoSAttackWithLinkedHashSet() { + final Set set = new LinkedHashSet(); + Set s1 = set; + Set s2 = new LinkedHashSet(); + for (int i = 0; i < 30; i++) { + final Set t1 = new LinkedHashSet(); + final Set t2 = new LinkedHashSet(); + t1.add("a"); + t2.add("b"); + s1.add(t1); + s1.add(t2); + s2.add(t2); + s2.add(t1); + s1 = t1; + s2 = t2; + } + + xstream.setCollectionUpdateLimit(5); + final String xml = xstream.toXML(set); + try { + xstream.fromXML(xml); + fail("Thrown " + InputManipulationException.class.getName() + " expected"); + } catch (final InputManipulationException e) { + assertTrue("Limit expected in message", e.getMessage().contains("exceeds 5 seconds")); + } + } + + public void testDoSAttackWithHashMap() { + final Map map = new HashMap(); + Map m1 = map; + Map m2 = new HashMap(); + for (int i = 0; i < 25; i++) { + final Map t1 = new HashMap(); + final Map t2 = new HashMap(); + t1.put("a", "b"); + t2.put("c", "d"); + m1.put(t1, t2); + m1.put(t2, t1); + m2.put(t2, t1); + m2.put(t1, t2); + m1 = t1; + m2 = t2; + } + xstream.setCollectionUpdateLimit(5); + + final String xml = xstream.toXML(map); + try { + xstream.fromXML(xml); + fail("Thrown " + InputManipulationException.class.getName() + " expected"); + } catch (InputManipulationException e) { + assertTrue("Limit expected in message", e.getMessage().contains("exceeds 5 seconds")); + } + } + + public void testDoSAttackWithLinkedHashMap() { + final Map map = new LinkedHashMap(); + Map m1 = map; + Map m2 = new LinkedHashMap(); + for (int i = 0; i < 25; i++) { + final Map t1 = new LinkedHashMap(); + final Map t2 = new LinkedHashMap(); + t1.put("a", "b"); + t2.put("c", "d"); + m1.put(t1, t2); + m1.put(t2, t1); + m2.put(t2, t1); + m2.put(t1, t2); + m1 = t1; + m2 = t2; + } + + xstream.setCollectionUpdateLimit(5); + final String xml = xstream.toXML(map); + try { + xstream.fromXML(xml); + fail("Thrown " + InputManipulationException.class.getName() + " expected"); + } catch (final InputManipulationException e) { + assertTrue("Limit expected in message", e.getMessage().contains("exceeds 5 seconds")); + } + } + + public void testDoSAttackWithHashtable() { + final Map map = new Hashtable(); + Map m1 = map; + Map m2 = new Hashtable(); + for (int i = 0; i < 100; i++) { + final Map t1 = new Hashtable(); + final Map t2 = new Hashtable(); + t1.put("a", "b"); + t2.put("c", "d"); + m1.put(t1, t2); + m1.put(t2, t1); + m2.put(t2, t1); + m2.put(t1, t2); + m1 = t1; + m2 = t2; + } + + xstream.setCollectionUpdateLimit(5); + final String xml = xstream.toXML(map); + try { + xstream.fromXML(xml); + fail("Thrown " + InputManipulationException.class.getName() + " expected"); + } catch (final InputManipulationException e) { + assertTrue("Limit expected in message", e.getMessage().contains("exceeds 5 seconds")); + } + } }