|
0 |
From: Markus Koschany <apo@debian.org>
|
|
1 |
Date: Tue, 1 Dec 2020 23:11:04 +0100
|
|
2 |
Subject: CVE-2020-26217
|
|
3 |
|
|
4 |
Origin: https://github.com/x-stream/xstream/commit/6ec68c4e4192faec64f350e9449f44bc120c813b
|
|
5 |
Origin: https://github.com/x-stream/xstream/commit/51abe602e09016c8e43e91325a15226022f4da46
|
|
6 |
Origin: https://github.com/x-stream/xstream/commit/0fec095d534126931c99fd38e9c6d41f5c685c1a
|
|
7 |
---
|
|
8 |
.../src/java/com/thoughtworks/xstream/XStream.java | 40 ++----
|
|
9 |
.../acceptance/SecurityVulnerabilityTest.java | 136 ++++++++++++++++-----
|
|
10 |
2 files changed, 121 insertions(+), 55 deletions(-)
|
|
11 |
|
|
12 |
diff --git a/xstream/src/java/com/thoughtworks/xstream/XStream.java b/xstream/src/java/com/thoughtworks/xstream/XStream.java
|
|
13 |
index a088877..0ae38b6 100644
|
|
14 |
--- a/xstream/src/java/com/thoughtworks/xstream/XStream.java
|
|
15 |
+++ b/xstream/src/java/com/thoughtworks/xstream/XStream.java
|
|
16 |
@@ -1,6 +1,6 @@
|
|
17 |
/*
|
|
18 |
* Copyright (C) 2003, 2004, 2005, 2006 Joe Walnes.
|
|
19 |
- * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018 XStream Committers.
|
|
20 |
+ * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2020 XStream Committers.
|
|
21 |
* All rights reserved.
|
|
22 |
*
|
|
23 |
* The software in this package is published under the terms of the BSD
|
|
24 |
@@ -36,6 +36,7 @@ import java.net.URL;
|
|
25 |
import java.nio.charset.Charset;
|
|
26 |
import java.text.DecimalFormatSymbols;
|
|
27 |
import java.util.ArrayList;
|
|
28 |
+import java.util.Arrays;
|
|
29 |
import java.util.BitSet;
|
|
30 |
import java.util.Calendar;
|
|
31 |
import java.util.Collection;
|
|
32 |
@@ -65,10 +66,8 @@ import com.thoughtworks.xstream.converters.Converter;
|
|
33 |
import com.thoughtworks.xstream.converters.ConverterLookup;
|
|
34 |
import com.thoughtworks.xstream.converters.ConverterRegistry;
|
|
35 |
import com.thoughtworks.xstream.converters.DataHolder;
|
|
36 |
-import com.thoughtworks.xstream.converters.MarshallingContext;
|
|
37 |
import com.thoughtworks.xstream.converters.SingleValueConverter;
|
|
38 |
import com.thoughtworks.xstream.converters.SingleValueConverterWrapper;
|
|
39 |
-import com.thoughtworks.xstream.converters.UnmarshallingContext;
|
|
40 |
import com.thoughtworks.xstream.converters.basic.BigDecimalConverter;
|
|
41 |
import com.thoughtworks.xstream.converters.basic.BigIntegerConverter;
|
|
42 |
import com.thoughtworks.xstream.converters.basic.BooleanConverter;
|
|
43 |
@@ -355,6 +354,8 @@ public class XStream {
|
|
44 |
|
|
45 |
private static final String ANNOTATION_MAPPER_TYPE = "com.thoughtworks.xstream.mapper.AnnotationMapper";
|
|
46 |
private static final Pattern IGNORE_ALL = Pattern.compile(".*");
|
|
47 |
+ private static final Pattern LAZY_ITERATORS = Pattern.compile(".*\\$LazyIterator");
|
|
48 |
+ private static final Pattern JAVAX_CRYPTO = Pattern.compile("javax\\.crypto\\..*");
|
|
49 |
|
|
50 |
/**
|
|
51 |
* Constructs a default XStream.
|
|
52 |
@@ -697,6 +698,12 @@ public class XStream {
|
|
53 |
}
|
|
54 |
|
|
55 |
addPermission(AnyTypePermission.ANY);
|
|
56 |
+ denyTypes(new String[]{"java.beans.EventHandler", "javax.imageio.ImageIO$ContainsFilter"});
|
|
57 |
+ denyTypes(new Class[]{ java.lang.ProcessBuilder.class,
|
|
58 |
+ java.beans.EventHandler.class, java.lang.ProcessBuilder.class,
|
|
59 |
+ java.lang.Void.class, void.class });
|
|
60 |
+ denyTypesByRegExp(new Pattern[] {LAZY_ITERATORS, JAVAX_CRYPTO});
|
|
61 |
+ allowTypeHierarchy(Exception.class);
|
|
62 |
securityInitialized = false;
|
|
63 |
}
|
|
64 |
|
|
65 |
@@ -962,7 +969,6 @@ public class XStream {
|
|
66 |
registerConverter(
|
|
67 |
new SerializableConverter(mapper, reflectionProvider, classLoaderReference), PRIORITY_LOW);
|
|
68 |
registerConverter(new ExternalizableConverter(mapper, classLoaderReference), PRIORITY_LOW);
|
|
69 |
- registerConverter(new InternalBlackList(), PRIORITY_LOW);
|
|
70 |
|
|
71 |
registerConverter(new NullConverter(), PRIORITY_VERY_HIGH);
|
|
72 |
registerConverter(new IntConverter(), PRIORITY_NORMAL);
|
|
73 |
@@ -1482,7 +1488,8 @@ public class XStream {
|
|
74 |
try {
|
|
75 |
if (!securityInitialized && !securityWarningGiven) {
|
|
76 |
securityWarningGiven = true;
|
|
77 |
- System.err.println("Security framework of XStream not initialized, XStream is probably vulnerable.");
|
|
78 |
+ System.err
|
|
79 |
+ .println("Security framework of XStream not explicitly initialized, using predefined black list on your own risk.");
|
|
80 |
}
|
|
81 |
return marshallingStrategy.unmarshal(
|
|
82 |
root, reader, dataHolder, converterLookup, mapper);
|
|
83 |
@@ -2360,7 +2367,7 @@ public class XStream {
|
|
84 |
*/
|
|
85 |
public void addPermission(TypePermission permission) {
|
|
86 |
if (securityMapper != null) {
|
|
87 |
- securityInitialized = true;
|
|
88 |
+ securityInitialized |= permission.equals(NoTypePermission.NONE) || permission.equals(AnyTypePermission.ANY);
|
|
89 |
securityMapper.addPermission(permission);
|
|
90 |
}
|
|
91 |
}
|
|
92 |
@@ -2539,25 +2546,4 @@ public class XStream {
|
|
93 |
super(message);
|
|
94 |
}
|
|
95 |
}
|
|
96 |
-
|
|
97 |
- private class InternalBlackList implements Converter {
|
|
98 |
-
|
|
99 |
- public boolean canConvert(final Class type) {
|
|
100 |
- return (type == void.class || type == Void.class)
|
|
101 |
- || (!securityInitialized
|
|
102 |
- && type != null
|
|
103 |
- && (type.getName().equals("java.beans.EventHandler")
|
|
104 |
- || type.getName().endsWith("$LazyIterator")
|
|
105 |
- || type.getName().startsWith("javax.crypto.")));
|
|
106 |
- }
|
|
107 |
-
|
|
108 |
- public void marshal(final Object source, final HierarchicalStreamWriter writer,
|
|
109 |
- final MarshallingContext context) {
|
|
110 |
- throw new ConversionException("Security alert. Marshalling rejected.");
|
|
111 |
- }
|
|
112 |
-
|
|
113 |
- public Object unmarshal(final HierarchicalStreamReader reader, final UnmarshallingContext context) {
|
|
114 |
- throw new ConversionException("Security alert. Unmarshalling rejected.");
|
|
115 |
- }
|
|
116 |
- }
|
|
117 |
}
|
|
118 |
diff --git a/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java b/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java
|
|
119 |
index 85eaf1c..44b0015 100644
|
|
120 |
--- a/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java
|
|
121 |
+++ b/xstream/src/test/com/thoughtworks/acceptance/SecurityVulnerabilityTest.java
|
|
122 |
@@ -11,13 +11,15 @@
|
|
123 |
package com.thoughtworks.acceptance;
|
|
124 |
|
|
125 |
import java.beans.EventHandler;
|
|
126 |
+import java.util.Iterator;
|
|
127 |
|
|
128 |
import com.thoughtworks.xstream.XStream;
|
|
129 |
import com.thoughtworks.xstream.XStreamException;
|
|
130 |
import com.thoughtworks.xstream.converters.ConversionException;
|
|
131 |
-import com.thoughtworks.xstream.converters.reflection.ReflectionConverter;
|
|
132 |
+import com.thoughtworks.xstream.core.JVM;
|
|
133 |
+import com.thoughtworks.xstream.security.AnyTypePermission;
|
|
134 |
import com.thoughtworks.xstream.security.ForbiddenClassException;
|
|
135 |
-import com.thoughtworks.xstream.security.ProxyTypePermission;
|
|
136 |
+import com.thoughtworks.xstream.security.NoTypePermission;
|
|
137 |
|
|
138 |
|
|
139 |
/**
|
|
140 |
@@ -31,21 +33,22 @@ public class SecurityVulnerabilityTest extends AbstractAcceptanceTest {
|
|
141 |
super.setUp();
|
|
142 |
BUFFER.setLength(0);
|
|
143 |
xstream.alias("runnable", Runnable.class);
|
|
144 |
- xstream.allowTypeHierarchy(Runnable.class);
|
|
145 |
- xstream.addPermission(ProxyTypePermission.PROXIES);
|
|
146 |
+ }
|
|
147 |
+
|
|
148 |
+ protected void setupSecurity(XStream xstream) {
|
|
149 |
}
|
|
150 |
|
|
151 |
public void testCannotInjectEventHandler() {
|
|
152 |
final String xml = ""
|
|
153 |
- + "<string class='runnable-array'>\n"
|
|
154 |
- + " <dynamic-proxy>\n"
|
|
155 |
- + " <interface>java.lang.Runnable</interface>\n"
|
|
156 |
- + " <handler class='java.beans.EventHandler'>\n"
|
|
157 |
- + " <target class='com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec'/>\n"
|
|
158 |
- + " <action>exec</action>\n"
|
|
159 |
- + " </handler>\n"
|
|
160 |
- + " </dynamic-proxy>\n"
|
|
161 |
- + "</string>";
|
|
162 |
+ + "<string class='runnable-array'>\n"
|
|
163 |
+ + " <dynamic-proxy>\n"
|
|
164 |
+ + " <interface>java.lang.Runnable</interface>\n"
|
|
165 |
+ + " <handler class='java.beans.EventHandler'>\n"
|
|
166 |
+ + " <target class='com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec'/>\n"
|
|
167 |
+ + " <action>exec</action>\n"
|
|
168 |
+ + " </handler>\n"
|
|
169 |
+ + " </dynamic-proxy>\n"
|
|
170 |
+ + "</string>";
|
|
171 |
|
|
172 |
try {
|
|
173 |
xstream.fromXML(xml);
|
|
174 |
@@ -57,7 +60,6 @@ public class SecurityVulnerabilityTest extends AbstractAcceptanceTest {
|
|
175 |
}
|
|
176 |
|
|
177 |
public void testCannotInjectEventHandlerWithUnconfiguredSecurityFramework() {
|
|
178 |
- xstream = new XStream(createDriver());
|
|
179 |
xstream.alias("runnable", Runnable.class);
|
|
180 |
final String xml = ""
|
|
181 |
+ "<string class='runnable-array'>\n"
|
|
182 |
@@ -74,26 +76,24 @@ public class SecurityVulnerabilityTest extends AbstractAcceptanceTest {
|
|
183 |
xstream.fromXML(xml);
|
|
184 |
fail("Thrown " + XStreamException.class.getName() + " expected");
|
|
185 |
} catch (final XStreamException e) {
|
|
186 |
- assertTrue(e.getMessage().indexOf(EventHandler.class.getName())>=0);
|
|
187 |
+ assertTrue(e.getMessage().indexOf(EventHandler.class.getName()) >= 0);
|
|
188 |
}
|
|
189 |
assertEquals(0, BUFFER.length());
|
|
190 |
}
|
|
191 |
|
|
192 |
public void testExplicitlyConvertEventHandler() {
|
|
193 |
final String xml = ""
|
|
194 |
- + "<string class='runnable-array'>\n"
|
|
195 |
- + " <dynamic-proxy>\n"
|
|
196 |
- + " <interface>java.lang.Runnable</interface>\n"
|
|
197 |
- + " <handler class='java.beans.EventHandler'>\n"
|
|
198 |
- + " <target class='com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec'/>\n"
|
|
199 |
- + " <action>exec</action>\n"
|
|
200 |
- + " </handler>\n"
|
|
201 |
- + " </dynamic-proxy>\n"
|
|
202 |
- + "</string>";
|
|
203 |
+ + "<string class='runnable-array'>\n"
|
|
204 |
+ + " <dynamic-proxy>\n"
|
|
205 |
+ + " <interface>java.lang.Runnable</interface>\n"
|
|
206 |
+ + " <handler class='java.beans.EventHandler'>\n"
|
|
207 |
+ + " <target class='com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec'/>\n"
|
|
208 |
+ + " <action>exec</action>\n"
|
|
209 |
+ + " </handler>\n"
|
|
210 |
+ + " </dynamic-proxy>\n"
|
|
211 |
+ + "</string>";
|
|
212 |
|
|
213 |
xstream.allowTypes(new Class[]{EventHandler.class});
|
|
214 |
- xstream.registerConverter(new ReflectionConverter(xstream.getMapper(), xstream
|
|
215 |
- .getReflectionProvider(), EventHandler.class));
|
|
216 |
|
|
217 |
final Runnable[] array = (Runnable[])xstream.fromXML(xml);
|
|
218 |
assertEquals(0, BUFFER.length());
|
|
219 |
@@ -101,6 +101,71 @@ public class SecurityVulnerabilityTest extends AbstractAcceptanceTest {
|
|
220 |
assertEquals("Executed!", BUFFER.toString());
|
|
221 |
}
|
|
222 |
|
|
223 |
+ public void testCannotInjectConvertImageIOContainsFilterWithUnconfiguredSecurityFramework() {
|
|
224 |
+ if (JVM.isVersion(7)) {
|
|
225 |
+ final String xml = ""
|
|
226 |
+ + "<string class='javax.imageio.spi.FilterIterator'>\n"
|
|
227 |
+ + " <iter class='java.util.ArrayList$Itr'>\n"
|
|
228 |
+ + " <cursor>0</cursor>\n"
|
|
229 |
+ + " <lastRet>1</lastRet>\n"
|
|
230 |
+ + " <expectedModCount>1</expectedModCount>\n"
|
|
231 |
+ + " <outer-class>\n"
|
|
232 |
+ + " <com.thoughtworks.acceptance.SecurityVulnerabilityTest_-Exec/>\n"
|
|
233 |
+ + " </outer-class>\n"
|
|
234 |
+ + " </iter>\n"
|
|
235 |
+ + " <filter class='javax.imageio.ImageIO$ContainsFilter'>\n"
|
|
236 |
+ + " <method>\n"
|
|
237 |
+ + " <class>com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec</class>\n"
|
|
238 |
+ + " <name>exec</name>\n"
|
|
239 |
+ + " <parameter-types/>\n"
|
|
240 |
+ + " </method>\n"
|
|
241 |
+ + " <name>exec</name>\n"
|
|
242 |
+ + " </filter>\n"
|
|
243 |
+ + " <next/>\n"
|
|
244 |
+ + "</string>";
|
|
245 |
+
|
|
246 |
+ try {
|
|
247 |
+ xstream.fromXML(xml);
|
|
248 |
+ fail("Thrown " + XStreamException.class.getName() + " expected");
|
|
249 |
+ } catch (final XStreamException e) {
|
|
250 |
+ assertTrue(e.getMessage().indexOf("javax.imageio.ImageIO$ContainsFilter") >= 0);
|
|
251 |
+ }
|
|
252 |
+ assertEquals(0, BUFFER.length());
|
|
253 |
+ }
|
|
254 |
+ }
|
|
255 |
+
|
|
256 |
+ public void testExplicitlyConvertImageIOContainsFilter() {
|
|
257 |
+ if (JVM.isVersion(7)) {
|
|
258 |
+ final String xml = ""
|
|
259 |
+ + "<string class='javax.imageio.spi.FilterIterator'>\n"
|
|
260 |
+ + " <iter class='java.util.ArrayList$Itr'>\n"
|
|
261 |
+ + " <cursor>0</cursor>\n"
|
|
262 |
+ + " <lastRet>1</lastRet>\n"
|
|
263 |
+ + " <expectedModCount>1</expectedModCount>\n"
|
|
264 |
+ + " <outer-class>\n"
|
|
265 |
+ + " <com.thoughtworks.acceptance.SecurityVulnerabilityTest_-Exec/>\n"
|
|
266 |
+ + " </outer-class>\n"
|
|
267 |
+ + " </iter>\n"
|
|
268 |
+ + " <filter class='javax.imageio.ImageIO$ContainsFilter'>\n"
|
|
269 |
+ + " <method>\n"
|
|
270 |
+ + " <class>com.thoughtworks.acceptance.SecurityVulnerabilityTest$Exec</class>\n"
|
|
271 |
+ + " <name>exec</name>\n"
|
|
272 |
+ + " <parameter-types/>\n"
|
|
273 |
+ + " </method>\n"
|
|
274 |
+ + " <name>exec</name>\n"
|
|
275 |
+ + " </filter>\n"
|
|
276 |
+ + " <next/>\n"
|
|
277 |
+ + "</string>";
|
|
278 |
+
|
|
279 |
+ xstream.allowTypes(new String[]{"javax.imageio.ImageIO$ContainsFilter"});
|
|
280 |
+
|
|
281 |
+ final Iterator iterator = (Iterator)xstream.fromXML(xml);
|
|
282 |
+ assertEquals(0, BUFFER.length());
|
|
283 |
+ iterator.next();
|
|
284 |
+ assertEquals("Executed!", BUFFER.toString());
|
|
285 |
+ }
|
|
286 |
+ }
|
|
287 |
+
|
|
288 |
public static class Exec {
|
|
289 |
|
|
290 |
public void exec() {
|
|
291 |
@@ -109,6 +174,8 @@ public class SecurityVulnerabilityTest extends AbstractAcceptanceTest {
|
|
292 |
}
|
|
293 |
|
|
294 |
public void testDeniedInstanceOfVoid() {
|
|
295 |
+ xstream.addPermission(AnyTypePermission.ANY); // clear out defaults
|
|
296 |
+ xstream.denyTypes(new Class[] { void.class, Void.class });
|
|
297 |
try {
|
|
298 |
xstream.fromXML("<void/>");
|
|
299 |
fail("Thrown " + ForbiddenClassException.class.getName() + " expected");
|
|
300 |
@@ -118,12 +185,25 @@ public class SecurityVulnerabilityTest extends AbstractAcceptanceTest {
|
|
301 |
}
|
|
302 |
|
|
303 |
public void testAllowedInstanceOfVoid() {
|
|
304 |
- xstream.allowTypes(new Class[] { void.class, Void.class });
|
|
305 |
+ xstream.allowTypes(new Class[]{void.class, Void.class});
|
|
306 |
try {
|
|
307 |
xstream.fromXML("<void/>");
|
|
308 |
fail("Thrown " + ConversionException.class.getName() + " expected");
|
|
309 |
} catch (final ConversionException e) {
|
|
310 |
- assertEquals("void", e.get("required-type"));
|
|
311 |
+ assertEquals("void", e.get("construction-type"));
|
|
312 |
+ }
|
|
313 |
+ }
|
|
314 |
+
|
|
315 |
+ public static class LazyIterator {
|
|
316 |
+ }
|
|
317 |
+
|
|
318 |
+ public void testInstanceOfLazyIterator() {
|
|
319 |
+ xstream.alias("lazy-iterator", LazyIterator.class);
|
|
320 |
+ try {
|
|
321 |
+ xstream.fromXML("<lazy-iterator/>");
|
|
322 |
+ fail("Thrown " + ForbiddenClassException.class.getName() + " expected");
|
|
323 |
+ } catch (final ForbiddenClassException e) {
|
|
324 |
+ // OK
|
|
325 |
}
|
|
326 |
}
|
|
327 |
}
|