165 lines
7.3 KiB
Diff
165 lines
7.3 KiB
Diff
From 51abe602e09016c8e43e91325a15226022f4da46 Mon Sep 17 00:00:00 2001
|
|
From: joehni <joerg.schaible@gmx.de>
|
|
Date: Wed, 2 Sep 2020 00:38:51 +0200
|
|
Subject: [PATCH] New predefined blacklist avoids vulnerability due to improper
|
|
setup of the security framework. Closes #207.
|
|
|
|
---
|
|
.../com/thoughtworks/xstream/XStream.java | 37 +++++--------------
|
|
.../acceptance/SecurityVulnerabilityTest.java | 29 +++++++++++++--
|
|
2 files changed, 36 insertions(+), 30 deletions(-)
|
|
|
|
diff --git a/xstream/src/java/com/thoughtworks/xstream/XStream.java b/xstream/src/java/com/thoughtworks/xstream/XStream.java
|
|
index 4eb2e33a..72e32c6e 100644
|
|
--- a/xstream/src/java/com/thoughtworks/xstream/XStream.java
|
|
+++ b/xstream/src/java/com/thoughtworks/xstream/XStream.java
|
|
@@ -1,6 +1,6 @@
|
|
/*
|
|
* Copyright (C) 2003, 2004, 2005, 2006 Joe Walnes.
|
|
- * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018 XStream Committers.
|
|
+ * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2020 XStream Committers.
|
|
* All rights reserved.
|
|
*
|
|
* The software in this package is published under the terms of the BSD
|
|
@@ -36,6 +36,7 @@
|
|
import java.nio.charset.Charset;
|
|
import java.text.DecimalFormatSymbols;
|
|
import java.util.ArrayList;
|
|
+import java.util.Arrays;
|
|
import java.util.BitSet;
|
|
import java.util.Calendar;
|
|
import java.util.Collection;
|
|
@@ -65,10 +66,8 @@
|
|
import com.thoughtworks.xstream.converters.ConverterLookup;
|
|
import com.thoughtworks.xstream.converters.ConverterRegistry;
|
|
import com.thoughtworks.xstream.converters.DataHolder;
|
|
-import com.thoughtworks.xstream.converters.MarshallingContext;
|
|
import com.thoughtworks.xstream.converters.SingleValueConverter;
|
|
import com.thoughtworks.xstream.converters.SingleValueConverterWrapper;
|
|
-import com.thoughtworks.xstream.converters.UnmarshallingContext;
|
|
import com.thoughtworks.xstream.converters.basic.BigDecimalConverter;
|
|
import com.thoughtworks.xstream.converters.basic.BigIntegerConverter;
|
|
import com.thoughtworks.xstream.converters.basic.BooleanConverter;
|
|
@@ -355,6 +354,8 @@
|
|
|
|
private static final String ANNOTATION_MAPPER_TYPE = "com.thoughtworks.xstream.mapper.AnnotationMapper";
|
|
private static final Pattern IGNORE_ALL = Pattern.compile(".*");
|
|
+ private static final Pattern LAZY_ITERATORS = Pattern.compile(".*\\$LazyIterator");
|
|
+ private static final Pattern JAVAX_CRYPTO = Pattern.compile("javax\\.crypto\\..*");
|
|
|
|
/**
|
|
* Constructs a default XStream.
|
|
@@ -697,6 +698,9 @@ protected void setupSecurity() {
|
|
}
|
|
|
|
addPermission(AnyTypePermission.ANY);
|
|
+ denyTypes(new String[]{"java.beans.EventHandler"});
|
|
+ denyTypesByRegExp(new Pattern[] {LAZY_ITERATORS, JAVAX_CRYPTO});
|
|
+ allowTypeHierarchy(Exception.class);
|
|
securityInitialized = false;
|
|
}
|
|
|
|
@@ -962,7 +966,6 @@ protected void setupConverters() {
|
|
registerConverter(
|
|
new SerializableConverter(mapper, reflectionProvider, classLoaderReference), PRIORITY_LOW);
|
|
registerConverter(new ExternalizableConverter(mapper, classLoaderReference), PRIORITY_LOW);
|
|
- registerConverter(new InternalBlackList(), PRIORITY_LOW);
|
|
|
|
registerConverter(new NullConverter(), PRIORITY_VERY_HIGH);
|
|
registerConverter(new IntConverter(), PRIORITY_NORMAL);
|
|
@@ -1482,7 +1485,8 @@ public Object unmarshal(HierarchicalStreamReader reader, Object root, DataHolder
|
|
try {
|
|
if (!securityInitialized && !securityWarningGiven) {
|
|
securityWarningGiven = true;
|
|
- System.err.println("Security framework of XStream not initialized, XStream is probably vulnerable.");
|
|
+ System.err
|
|
+ .println("Security framework of XStream not explicitly initialized, using predefined black list on your own risk.");
|
|
}
|
|
return marshallingStrategy.unmarshal(
|
|
root, reader, dataHolder, converterLookup, mapper);
|
|
@@ -2360,7 +2364,7 @@ public void autodetectAnnotations(boolean mode) {
|
|
*/
|
|
public void addPermission(TypePermission permission) {
|
|
if (securityMapper != null) {
|
|
- securityInitialized = true;
|
|
+ securityInitialized |= permission.equals(NoTypePermission.NONE) || permission.equals(AnyTypePermission.ANY);
|
|
securityMapper.addPermission(permission);
|
|
}
|
|
}
|
|
@@ -2539,25 +2543,4 @@ public InitializationException(String message) {
|
|
super(message);
|
|
}
|
|
}
|
|
-
|
|
- private class InternalBlackList implements Converter {
|
|
-
|
|
- public boolean canConvert(final Class type) {
|
|
- return (type == void.class || type == Void.class)
|
|
- || (!securityInitialized
|
|
- && type != null
|
|
- && (type.getName().equals("java.beans.EventHandler")
|
|
- || type.getName().endsWith("$LazyIterator")
|
|
- || type.getName().startsWith("javax.crypto.")));
|
|
- }
|
|
-
|
|
- public void marshal(final Object source, final HierarchicalStreamWriter writer,
|
|
- final MarshallingContext context) {
|
|
- throw new ConversionException("Security alert. Marshalling rejected.");
|
|
- }
|
|
-
|
|
- public Object unmarshal(final HierarchicalStreamReader reader, final UnmarshallingContext context) {
|
|
- throw new ConversionException("Security alert. Unmarshalling rejected.");
|
|
- }
|
|
- }
|
|
}
|
|
diff --git a/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java b/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java
|
|
index c7d7ebe4..18379276 100644
|
|
--- a/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java
|
|
+++ b/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java
|
|
@@ -16,6 +16,7 @@
|
|
import com.thoughtworks.xstream.XStreamException;
|
|
import com.thoughtworks.xstream.converters.ConversionException;
|
|
import com.thoughtworks.xstream.converters.reflection.ReflectionConverter;
|
|
+import com.thoughtworks.xstream.security.AnyTypePermission;
|
|
import com.thoughtworks.xstream.security.ForbiddenClassException;
|
|
import com.thoughtworks.xstream.security.ProxyTypePermission;
|
|
|
|
@@ -108,6 +109,15 @@ public void exec() {
|
|
}
|
|
}
|
|
|
|
+ public void testInstanceOfVoid() {
|
|
+ try {
|
|
+ xstream.fromXML("<void/>");
|
|
+ fail("Thrown " + ConversionException.class.getName() + " expected");
|
|
+ } catch (final ConversionException e) {
|
|
+ assertEquals("void", e.get("construction-type"));
|
|
+ }
|
|
+ }
|
|
+
|
|
public void testDeniedInstanceOfVoid() {
|
|
try {
|
|
xstream.fromXML("<void/>");
|
|
@@ -123,7 +133,20 @@ public void testAllowedInstanceOfVoid() {
|
|
xstream.fromXML("<void/>");
|
|
fail("Thrown " + ConversionException.class.getName() + " expected");
|
|
} catch (final ConversionException e) {
|
|
- assertEquals("void", e.get("required-type"));
|
|
+ assertEquals("void", e.get("construction-type"));
|
|
+ }
|
|
+ }
|
|
+
|
|
+ public static class LazyIterator {
|
|
+ }
|
|
+
|
|
+ public void testInstanceOfLazyIterator() {
|
|
+ xstream.alias("lazy-iterator", LazyIterator.class);
|
|
+ try {
|
|
+ xstream.fromXML("<lazy-iterator/>");
|
|
+ fail("Thrown " + ForbiddenClassException.class.getName() + " expected");
|
|
+ } catch (final ForbiddenClassException e) {
|
|
+ // OK
|
|
}
|
|
}
|
|
}
|