[SECURITY-2602]

core/src/main/java/hudson/util/RobustCollectionConverter.java

@@ -33,9 +33,11 @@
 import com.thoughtworks.xstream.core.ClassLoaderReference;
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.mapper.Mapper;
+import com.thoughtworks.xstream.security.InputManipulationException;
 import java.util.Collection;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.logging.Logger;
 import jenkins.util.xstream.CriticalXStreamException;
 
 /**
@@ -83,9 +85,17 @@ protected void populateCollection(HierarchicalStreamReader reader, Unmarshalling
             reader.moveDown();
             try {
                 Object item = readBareItem(reader, context, collection);
+                long nanoNow = System.nanoTime();
                 collection.add(item);
+                XStream2SecurityUtils.checkForCollectionDoSAttack(context, nanoNow);
             } catch (CriticalXStreamException e) {
                 throw e;
+            } catch (InputManipulationException e) {
+                Logger.getLogger(RobustCollectionConverter.class.getName()).warning(
+                        "DoS detected and prevented. If the heuristic was too aggressive, " +
+                                "you can customize the behavior by setting the hudson.util.XStream2.collectionUpdateLimit system property. " +
+                                "See https://www.jenkins.io/redirect/xstream-dos-prevention for more information.");
+                throw new CriticalXStreamException(e);
             } catch (XStreamException | LinkageError e) {
                 RobustReflectionConverter.addErrorInContext(context, e);
             }

core/src/main/java/hudson/util/RobustMapConverter.java

@@ -29,15 +29,16 @@
 import com.thoughtworks.xstream.converters.collections.MapConverter;
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.mapper.Mapper;
+import com.thoughtworks.xstream.security.InputManipulationException;
 import java.util.Map;
+import java.util.logging.Logger;
 import jenkins.util.xstream.CriticalXStreamException;
 
 /**
  * Loads a {@link Map} while tolerating read errors on its keys and values.
  */
 @SuppressWarnings({"rawtypes", "unchecked"})
 final class RobustMapConverter extends MapConverter {
-
     private static final Object ERROR = new Object();
 
     RobustMapConverter(Mapper mapper) {
@@ -48,7 +49,17 @@ final class RobustMapConverter extends MapConverter {
         Object key = read(reader, context, map);
         Object value = read(reader, context, map);
         if (key != ERROR && value != ERROR) {
-            target.put(key, value);
+            try {
+                long nanoNow = System.nanoTime();
+                target.put(key, value);
+                XStream2SecurityUtils.checkForCollectionDoSAttack(context, nanoNow);
+            } catch (InputManipulationException e) {
+                Logger.getLogger(RobustMapConverter.class.getName()).warning(
+                        "DoS detected and prevented. If the heuristic was too aggressive, " +
+                                "you can customize the behavior by setting the hudson.util.XStream2.collectionUpdateLimit system property. " +
+                                "See https://www.jenkins.io/redirect/xstream-dos-prevention for more information.");
+                throw new CriticalXStreamException(e);
+            }
         }
     }
 

core/src/main/java/hudson/util/RobustReflectionConverter.java

@@ -42,6 +42,7 @@
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
 import com.thoughtworks.xstream.mapper.Mapper;
+import com.thoughtworks.xstream.security.InputManipulationException;
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.diagnosis.OldDataMonitor;
 import hudson.model.Saveable;
@@ -369,6 +370,12 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re
                 }
             } catch (CriticalXStreamException e) {
                 throw e;
+            } catch (InputManipulationException e) {
+                LOGGER.warning(
+                        "DoS detected and prevented. If the heuristic was too aggressive, " +
+                                "you can customize the behavior by setting the hudson.util.XStream2.collectionUpdateLimit system property. " +
+                                "See https://www.jenkins.io/redirect/xstream-dos-prevention for more information.");
+                throw new CriticalXStreamException(e);
             } catch (XStreamException e) {
                 if (critical) {
                     throw new CriticalXStreamException(e);

core/src/main/java/hudson/util/XStream2.java

@@ -82,7 +82,10 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import jenkins.model.Jenkins;
+import jenkins.util.SystemProperties;
 import jenkins.util.xstream.SafeURLConverter;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * {@link XStream} customized in various ways for Jenkins’ needs.
@@ -91,6 +94,17 @@
 public class XStream2 extends XStream {
 
     private static final Logger LOGGER = Logger.getLogger(XStream2.class.getName());
+    /**
+     * Determine what is the value (in seconds) of the "collectionUpdateLimit" added by XStream
+     * to protect against http://x-stream.github.io/CVE-2021-43859.html.
+     * It corresponds to the accumulated timeout when adding an item to a collection.
+     *
+     * Default: 5 seconds (in contrary to XStream default to 20 which is a bit too tolerant)
+     * If negative: disable the DoS protection
+     */
+    @Restricted(NoExternalUse.class)
+    public static final String COLLECTION_UPDATE_LIMIT_PROPERTY_NAME = XStream2.class.getName() + ".collectionUpdateLimit";
+    private static final int COLLECTION_UPDATE_LIMIT_DEFAULT_VALUE = 5;
 
     private RobustReflectionConverter reflectionConverter;
     private final ThreadLocal<Boolean> oldData = new ThreadLocal<>();
@@ -250,6 +264,9 @@ static String trimVersion(String version) {
     }
 
     private void init() {
+        int updateLimit = SystemProperties.getInteger(COLLECTION_UPDATE_LIMIT_PROPERTY_NAME, COLLECTION_UPDATE_LIMIT_DEFAULT_VALUE);
+        this.setCollectionUpdateLimit(updateLimit);
+
         // list up types that should be marshalled out like a value, without referential integrity tracking.
         addImmutableType(Result.class, false);
 

core/src/main/java/hudson/util/XStream2SecurityUtils.java

@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2021, 2022 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
+ */
+// Updated when included in Jenkins code by changing currentTimeMillis to nanoTime + comments
+
+package hudson.util;
+
+import com.thoughtworks.xstream.XStream;
+import com.thoughtworks.xstream.converters.ConversionException;
+import com.thoughtworks.xstream.converters.UnmarshallingContext;
+import com.thoughtworks.xstream.security.InputManipulationException;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+/**
+ * Strongly inspired by https://github.com/x-stream/xstream/blob/61a00fa225dc99488013869b57b772af8e2fea03/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java#L25
+ * and taking into account https://github.com/x-stream/xstream/issues/282
+ *
+ * Once the related issue is fixed, we will be able to use the regular method from XStream.
+ *
+ * @see com.thoughtworks.xstream.core.SecurityUtils
+ */
+@Restricted(NoExternalUse.class)
+public class XStream2SecurityUtils {
+    /**
+     * 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 startNano the nanoTime just before the element was added to the collection or map
+     * @since 1.4.19
+     */
+    public static void checkForCollectionDoSAttack(final UnmarshallingContext context, final long startNano) {
+        final int diff = (int) ((System.nanoTime() - startNano) / 1000_000_000);
+        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 + diff;
+                if (seconds > limit) {
+                    throw new InputManipulationException(
+                            "Denial of Service attack assumed. Adding elements to collections or maps exceeds " + limit + " seconds.");
+                }
+                context.put(XStream.COLLECTION_UPDATE_SECONDS, seconds);
+            }
+        }
+    }
+}

core/src/test/java/hudson/util/RobustCollectionConverterTest.java

@@ -0,0 +1,186 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2022 CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package hudson.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import com.thoughtworks.xstream.security.InputManipulationException;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import jenkins.util.xstream.CriticalXStreamException;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+
+public class RobustCollectionConverterTest {
+    @Test
+    public void workingByDefaultWithSimplePayload() {
+        XStream2 xstream2 = new XStream2();
+
+        Map<String, String> map = new HashMap<>();
+        map.put("a", "gsdf");
+        map.put("b", "");
+
+        Set<Object> set = new HashSet<>();
+        set.add(4L);
+        set.add(5.67);
+        set.add("tasd");
+        set.add('z');
+        set.add(Instant.now());
+
+        // to get an ArrayList and not a Arrays.ArrayList
+        List<Object> payload = new ArrayList<>(Arrays.asList(123, "abc", map, new Date(), set));
+
+        String xmlContent = xstream2.toXML(payload);
+
+        Object rawResult = xstream2.fromXML(xmlContent);
+        assertEquals(payload, rawResult);
+    }
+
+    /**
+     * As RobustCollectionConverter is the replacer of the default CollectionConverter
+     * We had to patch it in order to not be impacted by CVE-2021-43859
+     */
+    // force timeout to prevent DoS due to test in the case the DoS prevention is broken
+    @Test(timeout = 30 * 1000)
+    @Issue("SECURITY-2602")
+    public void dosIsPrevented_customProgrammaticallyTimeout() {
+        XStream2 xstream2 = new XStream2();
+
+        Set<Object> set = preparePayload();
+
+        xstream2.setCollectionUpdateLimit(3);
+        final String xml = xstream2.toXML(set);
+        try {
+            xstream2.fromXML(xml);
+            fail("Thrown " + CriticalXStreamException.class.getName() + " expected");
+        } catch (final CriticalXStreamException e) {
+            Throwable cause = e.getCause();
+            assertNotNull("A non-null cause of CriticalXStreamException is expected", cause);
+            assertTrue("Cause of CriticalXStreamException is expected to be InputManipulationException", cause instanceof InputManipulationException);
+            InputManipulationException ime = (InputManipulationException) cause;
+            assertTrue("Limit expected in message", ime.getMessage().contains("exceeds 3 seconds"));
+        }
+    }
+
+    @Test(timeout = 30 * 1000)
+    @Issue("SECURITY-2602")
+    public void dosIsPrevented_customPropertyTimeout() {
+        String currentValue = System.getProperty(XStream2.COLLECTION_UPDATE_LIMIT_PROPERTY_NAME);
+        try {
+            System.setProperty(XStream2.COLLECTION_UPDATE_LIMIT_PROPERTY_NAME, "4");
+
+            XStream2 xstream2 = new XStream2();
+
+            Set<Object> set = preparePayload();
+
+            final String xml = xstream2.toXML(set);
+            try {
+                xstream2.fromXML(xml);
+                fail("Thrown " + CriticalXStreamException.class.getName() + " expected");
+            } catch (final CriticalXStreamException e) {
+                Throwable cause = e.getCause();
+                assertNotNull("A non-null cause of CriticalXStreamException is expected", cause);
+                assertTrue("Cause of CriticalXStreamException is expected to be InputManipulationException", cause instanceof InputManipulationException);
+                InputManipulationException ime = (InputManipulationException) cause;
+                assertTrue("Limit expected in message", ime.getMessage().contains("exceeds 4 seconds"));
+            }
+        } finally {
+            if (currentValue == null) {
+                System.clearProperty(XStream2.COLLECTION_UPDATE_LIMIT_PROPERTY_NAME);
+            } else {
+                System.setProperty(XStream2.COLLECTION_UPDATE_LIMIT_PROPERTY_NAME, currentValue);
+            }
+        }
+    }
+
+    // force timeout to prevent DoS due to test in the case the DoS prevention is broken
+    @Test(timeout = 30 * 1000)
+    @Issue("SECURITY-2602")
+    public void dosIsPrevented_defaultTimeout() {
+        XStream2 xstream2 = new XStream2();
+
+        Set<Object> set = preparePayload();
+
+        final String xml = xstream2.toXML(set);
+        try {
+            xstream2.fromXML(xml);
+            fail("Thrown " + CriticalXStreamException.class.getName() + " expected");
+        } catch (final CriticalXStreamException e) {
+            Throwable cause = e.getCause();
+            assertNotNull("A non-null cause of CriticalXStreamException is expected", cause);
+            assertTrue("Cause of CriticalXStreamException is expected to be InputManipulationException", cause instanceof InputManipulationException);
+            InputManipulationException ime = (InputManipulationException) cause;
+            assertTrue("Limit expected in message", ime.getMessage().contains("exceeds 5 seconds"));
+        }
+    }
+
+    // Inspired by https://github.com/x-stream/xstream/commit/e8e88621ba1c85ac3b8620337dd672e0c0c3a846#diff-9fde4ecf1bb4dc9850c031cb161960d2e61e069b386fa0b3db0d57e0e9f5baa
+    // which seems to be inspired by https://owasp.org/www-community/vulnerabilities/Deserialization_of_untrusted_data
+    private Set<Object> preparePayload() {
+        /*
+            On a i7-1185G7@3.00GHz (2021)
+            Full test time:
+            max=10 => ~1s
+            max=15 => ~1s
+            max=20 => ~1s
+            max=25 => ~3s
+            max=26 => ~6s
+            max=27 => ~11s
+            max=28 => ~22s
+            max=29 => ~47s
+            max=30 => >1m30
+            max=32 => est. 6m
+
+            With the protection in place, each test is taking ~15 seconds before the protection triggers
+        */
+
+        final Set<Object> set = new HashSet<>();
+        Set<Object> s1 = set;
+        Set<Object> s2 = new HashSet<>();
+        for (int i = 0; i < 32; i++) {
+            final Set<Object> t1 = new HashSet<>();
+            final Set<Object> 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;
+        }
+        return set;
+    }
+}