packages/xstream
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
xstream/CVE-2021-43859.patch
wk333 71adb70575 Fix CVE-2021-43859 
(cherry picked from commit cf2ddb216806b30d6ab567b2652d76bf85a13a53)
2022-02-07 14:11:47 +08:00

878 lines
38 KiB
Diff
Raw Blame History

This file contains invisible Unicode characters

This file contains invisible Unicode characters that are indistinguishable to humans but may be processed differently by a computer. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

From e8e88621ba1c85ac3b8620337dd672e0c0c3a846 Mon Sep 17 00:00:00 2001
From: joehni <joerg.schaible@gmx.de>
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 @@
+<html>
+<!--
+ 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 23. December 2021 by Joerg Schaible
+ -->
+ <head>
+ <title>CVE-2021-43859</title>
+ </head>
+ <body>
+
+ <h2 id="vulnerability">Vulnerability</h2>
+
+ <p>CVE-2021-43859: XStream can cause a Denial of Service by injecting highly recursive collections or maps.</p>
+
+ <h2 id="affected_versions">Affected Versions</h2>
+
+ <p>All versions until and including version 1.4.18 are affected.</p>
+
+ <h2 id="description">Description</h2>
+
+ <p>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.</p>
+
+ <h2 id="reproduction">Steps to Reproduce</h2>
+
+ <p>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:</p>
+ <ul>
+ <li>java.util.HashMap</li>
+ <li>java.util.HashSet</li>
+ <li>java.util.Hashtable</li>
+ <li>java.util.LinkedHashMap</li>
+ <li>java.util.LinkedHashSet</li>
+ <li>java.util.Stack (older Java revisions only)</li>
+ <li>java.util.Vector (older Java revisions only)</li>
+ <li>Other third party collection implementations that use their element's hash code may also be affected</li>
+ </ul>
+ <p>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:</p>
+<div class="Source XML"><pre>&lt;set&gt;
+ &lt;set&gt;
+ &lt;string&gt;a&lt;/string&gt;
+ &lt;set&gt;
+ &lt;string&gt;a&lt;/string&gt;
+ &lt;set&gt;
+ &lt;string&gt;a&lt;/string&gt;
+ &lt;/set&gt;
+ &lt;set&gt;
+ &lt;string&gt;b&lt;/string&gt;
+ &lt;/set&gt;
+ &lt;/set&gt;
+ &lt;set&gt;
+ &lt;set reference=&quot;../../set/set&quot;/&gt;
+ &lt;string&gt;b&lt;/string&gt;
+ &lt;set reference=&quot;../../set/set[2]&quot;/&gt;
+ &lt;/set&gt;
+ &lt;/set&gt;
+ &lt;set&gt;
+ &lt;set reference=&quot;../../set/set&quot;/&gt;
+ &lt;string&gt;b&lt;/string&gt;
+ &lt;set reference=&quot;../../set/set[2]&quot;/&gt;
+ &lt;/set&gt;
+&lt;/set&gt;
+</pre></div>
+<div class="Source Java"><pre>XStream xstream = new XStream();
+xstream.fromXML(xml);
+</pre></div>
+ <p>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:</p>
+<div class="Source XML"><pre>&lt;map&gt;
+ &lt;entry&gt;
+ &lt;map&gt;
+ &lt;entry&gt;
+ &lt;string&gt;a&lt;/string&gt;
+ &lt;string&gt;b&lt;/string&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map&gt;
+ &lt;entry&gt;
+ &lt;string&gt;a&lt;/string&gt;
+ &lt;string&gt;b&lt;/string&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map&gt;
+ &lt;entry&gt;
+ &lt;string&gt;a&lt;/string&gt;
+ &lt;string&gt;b&lt;/string&gt;
+ &lt;/entry&gt;
+ &lt;/map&gt;
+ &lt;map&gt;
+ &lt;entry&gt;
+ &lt;string&gt;c&lt;/string&gt;
+ &lt;string&gt;d&lt;/string&gt;
+ &lt;/entry&gt;
+ &lt;/map&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map reference=&quot;../../entry[2]/map[2]&quot;/&gt;
+ &lt;map reference=&quot;../../entry[2]/map&quot;/&gt;
+ &lt;/entry&gt;
+ &lt;/map&gt;
+ &lt;map&gt;
+ &lt;entry&gt;
+ &lt;string&gt;c&lt;/string&gt;
+ &lt;string&gt;d&lt;/string&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map reference=&quot;../../../entry[2]/map&quot;/&gt;
+ &lt;map reference=&quot;../../../entry[2]/map[2]&quot;/&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map reference=&quot;../../../entry[2]/map[2]&quot;/&gt;
+ &lt;map reference=&quot;../../../entry[2]/map&quot;/&gt;
+ &lt;/entry&gt;
+ &lt;/map&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map reference=&quot;../../entry[2]/map[2]&quot;/&gt;
+ &lt;map reference=&quot;../../entry[2]/map&quot;/&gt;
+ &lt;/entry&gt;
+ &lt;/map&gt;
+ &lt;map&gt;
+ &lt;entry&gt;
+ &lt;string&gt;c&lt;/string&gt;
+ &lt;string&gt;d&lt;/string&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map reference=&quot;../../../entry[2]/map&quot;/&gt;
+ &lt;map reference=&quot;../../../entry[2]/map[2]&quot;/&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map reference=&quot;../../../entry[2]/map[2]&quot;/&gt;
+ &lt;map reference=&quot;../../../entry[2]/map&quot;/&gt;
+ &lt;/entry&gt;
+ &lt;/map&gt;
+ &lt;/entry&gt;
+ &lt;entry&gt;
+ &lt;map reference=&quot;../../entry[2]/map[2]&quot;/&gt;
+ &lt;map reference=&quot;../../entry[2]/map&quot;/&gt;
+ &lt;/entry&gt;
+&lt;/map&gt;
+</pre></div>
+<div class="Source Java"><pre>XStream xstream = new XStream();
+xstream.fromXML(xml);
+</pre></div>
+
+ <p>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.</p>
+
+ <p>Note, this example uses XML, but the attack can be performed for any supported format, that supports references, i.e.
+ JSON is not affected.</p>
+
+ <h2 id="impact">Impact</h2>
+
+ <p>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.</p>
+
+ <h2 id="workarounds">Workarounds</h2>
+
+ <p>If your object graph does not use referenced elements at all, you may simply set the NO_REFERENCE mode:</p>
+
+<div class="Source Java"><pre>XStream xstream = new XStream();
+xstream.setMode(XStream.NO_REFERENCES);
+</pre></div>
+
+ <p>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:</p>
+
+<div class="Source Java"><pre>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
+});
+</pre></div>
+
+ <p>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:</p>
+
+<div class="Source Java"><pre>xstream.addDefaultImplementation(java.util.TreeMap.class, java.util.Map.class);
+xstream.addDefaultImplementation(java.util.TreeSet.class, java.util.Set.class);
+</pre></div>
+
+ <p>However, this implies that your application does not care about the implementation of the map and all elements are comparable.</p>
+
+ <h2 id="credits">Credits</h2>
+
+ <p>r00t4dm at Cloud-Penetrating Arrow Lab found and reported the issue to XStream and provided the required information to
+ reproduce it.</p>
+
+ </body>
+ </html>
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.</p>
- <p class=highlight>Note: XStream supports other data formats than XML, e.g. JSON. Those formats can be used for
- the same attacks.</p>
+ <p class=highlight>Note: XStream supports other data formats than XML, e.g. JSON. Those formats can usually be used
+ for the same attacks.</p>
- <p>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.</p>
+ <p>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.</p>
<h2 id="CVEs">Documented Vulnerabilities</h2>
@@ -49,6 +49,14 @@ <h2 id="CVEs">Documented Vulnerabilities</h2>
<th>CVE</th>
<th>Description</th>
</tr>
+ <tr>
+ <th>Version 1.4.18</th>
+ <td></td>
+ </tr>
+ <tr>
+ <th><a href="CVE-2021-43859.html">CVE-2021-43859</a></th>
+ <td>XStream can cause a Denial of Service by injecting highly recursive collections or maps.</td>
+ </tr>
<tr>
<th>Version 1.4.17</th>
<td></td>
@@ -258,6 +266,16 @@ <h2 id="implicit">Implicit Security</h2>
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).</p>
+ <p>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).</p>
+
<h2 id="explicit">Explicit Security</h2>
    
<p>Starting with XStream 1.4.7, it is possible to define <a href="#framework">permissions</a> for types, to check
@@ -285,6 +303,16 @@ <h2 id="explicit">Explicit Security</h2>
<p class=highlight>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.</p>
+ <p>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:<div class="Source Java">
+<pre>// unmarshal element of collection
+long now = System.currentTimeMillis();
+// add element here, e.g. list.add(element);
+SecurityUtils.checkForCollectionDoSAttack(context, now);
+</pre></div></p>
+
<h2 id="validation">XML Validation</h2>
<p>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 @@
<page>CVE-2021-39152.html</page>
<page>CVE-2021-39153.html</page>
<page>CVE-2021-39154.html</page>
+ <page>CVE-2021-43859.html</page>
<page>CVE-2020-26217.html</page>
<page>CVE-2020-26258.html</page>
<page>CVE-2020-26259.html</page>
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&ouml;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&ouml;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&ouml;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&ouml;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("<void/>");
- 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"));
+ }
+ }
}
Reference in New Issue View Git Blame Copy Permalink