From fa54f72b935058ac38df6f8c0af3924be7961250 Mon Sep 17 00:00:00 2001 From: Himanshu Tanwar Date: Thu, 20 Mar 2025 11:31:31 +0530 Subject: [PATCH 1/5] add getLevel api from version 1.5 --- .../xstream/io/HierarchicalStreamReader.java | 13 ++++- .../xstream/io/ReaderWrapper.java | 7 ++- .../xstream/io/binary/BinaryStreamReader.java | 7 ++- .../xstream/io/binary/ReaderDepthState.java | 8 ++- .../io/xml/AbstractDocumentReader.java | 15 ++++-- .../xstream/io/xml/AbstractPullReader.java | 9 +++- .../MultipleObjectsInOneStreamTest.java | 38 +++++++++++++- .../xstream/io/xml/AbstractXMLReaderTest.java | 51 +++++++++++++++++-- 8 files changed, 130 insertions(+), 18 deletions(-) diff --git a/xstream/src/java/com/thoughtworks/xstream/io/HierarchicalStreamReader.java b/xstream/src/java/com/thoughtworks/xstream/io/HierarchicalStreamReader.java index 36e8d9da8..ddbc77f7c 100644 --- a/xstream/src/java/com/thoughtworks/xstream/io/HierarchicalStreamReader.java +++ b/xstream/src/java/com/thoughtworks/xstream/io/HierarchicalStreamReader.java @@ -6,7 +6,7 @@ * 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 07. March 2004 by Joe Walnes */ package com.thoughtworks.xstream.io; @@ -47,6 +47,15 @@ public interface HierarchicalStreamReader extends ErrorReporter { */ String getValue(); + /** + * Retrieve the current nesting level. The method counts the number of unbalanced calls to {@link #moveDown()} and + * {@link #moveUp()}. + * + * @return the current nesting level + * @since upcoming + */ + int getLevel(); + /** * Get the value of an attribute of the current node. *

@@ -63,7 +72,7 @@ public interface HierarchicalStreamReader extends ErrorReporter { *

*/ String getAttribute(int index); - + /** * Number of attributes in current node. */ diff --git a/xstream/src/java/com/thoughtworks/xstream/io/ReaderWrapper.java b/xstream/src/java/com/thoughtworks/xstream/io/ReaderWrapper.java index 38cbbf6a7..b61596691 100644 --- a/xstream/src/java/com/thoughtworks/xstream/io/ReaderWrapper.java +++ b/xstream/src/java/com/thoughtworks/xstream/io/ReaderWrapper.java @@ -6,7 +6,7 @@ * 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 10. April 2005 by Joe Walnes */ package com.thoughtworks.xstream.io; @@ -44,6 +44,11 @@ public String getNodeName() { return wrapped.getNodeName(); } + @Override + public int getLevel() { + return wrapped.getLevel(); + } + public String getValue() { return wrapped.getValue(); } diff --git a/xstream/src/java/com/thoughtworks/xstream/io/binary/BinaryStreamReader.java b/xstream/src/java/com/thoughtworks/xstream/io/binary/BinaryStreamReader.java index cd870cd00..9fbec6a45 100644 --- a/xstream/src/java/com/thoughtworks/xstream/io/binary/BinaryStreamReader.java +++ b/xstream/src/java/com/thoughtworks/xstream/io/binary/BinaryStreamReader.java @@ -6,7 +6,7 @@ * 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 04. June 2006 by Joe Walnes */ package com.thoughtworks.xstream.io.binary; @@ -148,6 +148,11 @@ public void moveUp() { pushBack(nextToken); } + @Override + public int getLevel() { + return depthState.getLevel(); + } + private Token readToken() { if (pushback == null) { try { diff --git a/xstream/src/java/com/thoughtworks/xstream/io/binary/ReaderDepthState.java b/xstream/src/java/com/thoughtworks/xstream/io/binary/ReaderDepthState.java index 6a27ce8c5..ba6fcf05d 100644 --- a/xstream/src/java/com/thoughtworks/xstream/io/binary/ReaderDepthState.java +++ b/xstream/src/java/com/thoughtworks/xstream/io/binary/ReaderDepthState.java @@ -6,7 +6,7 @@ * 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 04. June 2006 by Joe Walnes */ package com.thoughtworks.xstream.io.binary; @@ -34,6 +34,7 @@ private static class State { List attributes; boolean hasMoreChildren; State parent; + int level; } private static class Attribute { @@ -46,6 +47,7 @@ private static class Attribute { public void push() { State newState = new State(); newState.parent = current; + newState.level = getLevel() + 1; current = newState; } @@ -53,6 +55,10 @@ public void pop() { current = current.parent; } + public int getLevel() { + return current != null ? current.level : 0; + } + public String getName() { return current.name; } diff --git a/xstream/src/java/com/thoughtworks/xstream/io/xml/AbstractDocumentReader.java b/xstream/src/java/com/thoughtworks/xstream/io/xml/AbstractDocumentReader.java index 4cb562f3c..85a140fd6 100644 --- a/xstream/src/java/com/thoughtworks/xstream/io/xml/AbstractDocumentReader.java +++ b/xstream/src/java/com/thoughtworks/xstream/io/xml/AbstractDocumentReader.java @@ -6,7 +6,7 @@ * 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 24. April 2005 by Joe Walnes */ package com.thoughtworks.xstream.io.xml; @@ -29,7 +29,7 @@ protected AbstractDocumentReader(Object rootElement) { /** * @since 1.4 - */ + */ protected AbstractDocumentReader(Object rootElement, NameCoder nameCoder) { super(nameCoder); this.current = rootElement; @@ -40,11 +40,11 @@ protected AbstractDocumentReader(Object rootElement, NameCoder nameCoder) { /** * @since 1.2 * @deprecated As of 1.4, use {@link AbstractDocumentReader#AbstractDocumentReader(Object, NameCoder)} instead. - */ + */ protected AbstractDocumentReader(Object rootElement, XmlFriendlyReplacer replacer) { this(rootElement, (NameCoder)replacer); } - + protected abstract void reassignCurrentElement(Object current); protected abstract Object getParent(); protected abstract Object getChild(int index); @@ -84,9 +84,14 @@ public Iterator getAttributeNames() { return new AttributeNameIterator(this); } + @Override + public int getLevel() { + return pointers.size(); + } + public void appendErrors(ErrorWriter errorWriter) { } - + public Object getCurrent() { return this.current; } diff --git a/xstream/src/java/com/thoughtworks/xstream/io/xml/AbstractPullReader.java b/xstream/src/java/com/thoughtworks/xstream/io/xml/AbstractPullReader.java index be2d24719..c44b2968e 100644 --- a/xstream/src/java/com/thoughtworks/xstream/io/xml/AbstractPullReader.java +++ b/xstream/src/java/com/thoughtworks/xstream/io/xml/AbstractPullReader.java @@ -6,7 +6,7 @@ * 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 24. April 2005 by Joe Walnes */ package com.thoughtworks.xstream.io.xml; @@ -58,7 +58,7 @@ protected AbstractPullReader(NameCoder nameCoder) { protected AbstractPullReader(XmlFriendlyReplacer replacer) { this((NameCoder)replacer); } - + /** * Pull the next event from the stream. @@ -114,6 +114,11 @@ public void moveUp() { } } + @Override + public int getLevel() { + return elementStack.size(); + } + private void move() { final Event event = readEvent(); pool.push(event); diff --git a/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java b/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java index 557e7923d..e642008b0 100644 --- a/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java +++ b/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java @@ -6,7 +6,7 @@ * 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 09. December 2005 by Joe Walnes */ package com.thoughtworks.acceptance; @@ -24,6 +24,7 @@ import com.thoughtworks.xstream.io.ReaderWrapper; import com.thoughtworks.xstream.io.xml.MXParserDriver; import com.thoughtworks.xstream.io.xml.PrettyPrintWriter; +import com.thoughtworks.xstream.io.xml.Xpp3Driver; import com.thoughtworks.xstream.io.xml.XppReader; import com.thoughtworks.xstream.mapper.Mapper; import com.thoughtworks.xstream.testutil.CallLog; @@ -233,6 +234,41 @@ public void testFailSafeDeserialization() throws IOException, ClassNotFoundExcep ois.close(); } + public void testFailSafeDeserialization() throws IOException, ClassNotFoundException { + final String xml = "" + + "\n" + + " top\n" + + " \n" + + " first\n" + + " \n" + + " 1\n" + + " invalid\n" // deserialization will fail here + + " 3\n" + + " \n" + + " last\n" + + " \n" + + " bottom\n" + + ""; + + @SuppressWarnings("resource") + final HierarchicalStreamReader reader = new Xpp3Driver().createReader(new StringReader(xml)); + final ObjectInputStream ois = xstream.createObjectInputStream(reader); + final int level = reader.getLevel(); + assertEquals(1, level); + assertEquals("top", ois.readObject()); + try { + ois.readObject(); + fail("Thrown " + ConversionException.class.getName() + " expected"); + } catch (final ConversionException e) { + assertEquals(4, reader.getLevel()); + do { + reader.moveUp(); + } while (level != reader.getLevel()); + } + assertEquals("bottom", ois.readObject()); + ois.close(); + } + public void testObjectOutputStreamPropagatesCloseAndFlushEvents() throws IOException { // setup final CallLog log = new CallLog(); diff --git a/xstream/src/test/com/thoughtworks/xstream/io/xml/AbstractXMLReaderTest.java b/xstream/src/test/com/thoughtworks/xstream/io/xml/AbstractXMLReaderTest.java index 194e930dd..48b84b1cf 100644 --- a/xstream/src/test/com/thoughtworks/xstream/io/xml/AbstractXMLReaderTest.java +++ b/xstream/src/test/com/thoughtworks/xstream/io/xml/AbstractXMLReaderTest.java @@ -6,7 +6,7 @@ * 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 07. March 2004 by Joe Walnes */ package com.thoughtworks.xstream.io.xml; @@ -32,43 +32,52 @@ public void testStartsAtRootTag() throws Exception { public void testCanNavigateDownChildTagsByIndex() throws Exception { HierarchicalStreamReader xmlReader = createReader(""); + assertEquals(1, xmlReader.getLevel()); assertEquals("a", xmlReader.getNodeName()); assertTrue(xmlReader.hasMoreChildren()); xmlReader.moveDown(); // /a/b + assertEquals(2, xmlReader.getLevel()); assertEquals("b", xmlReader.getNodeName()); assertTrue(xmlReader.hasMoreChildren()); xmlReader.moveDown(); // a/b/ooh + assertEquals(3, xmlReader.getLevel()); assertEquals("ooh", xmlReader.getNodeName()); assertFalse(xmlReader.hasMoreChildren()); xmlReader.moveUp(); // a/b + assertEquals(2, xmlReader.getLevel()); assertFalse(xmlReader.hasMoreChildren()); xmlReader.moveUp(); // /a + assertEquals(1, xmlReader.getLevel()); assertTrue(xmlReader.hasMoreChildren()); xmlReader.moveDown(); // /a/b[2] + assertEquals(2, xmlReader.getLevel()); assertEquals("b", xmlReader.getNodeName()); assertTrue(xmlReader.hasMoreChildren()); xmlReader.moveDown(); // a/b[2]/aah + assertEquals(3, xmlReader.getLevel()); assertEquals("aah", xmlReader.getNodeName()); assertFalse(xmlReader.hasMoreChildren()); xmlReader.moveUp(); // a/b[2] + assertEquals(2, xmlReader.getLevel()); assertFalse(xmlReader.hasMoreChildren()); xmlReader.moveUp(); // a + assertEquals(1, xmlReader.getLevel()); assertFalse(xmlReader.hasMoreChildren()); } @@ -176,7 +185,7 @@ public void testExposesAttributesKeysAsIterator() throws Exception { } assertEquals(expected, actual); - // again, to check iteration is repeatable + // again, to check iteration is repeatable iterator = xmlReader.getAttributeNames(); while(iterator.hasNext()) { actual.add(iterator.next()); @@ -219,6 +228,8 @@ public void testExposesTextValueOfCurrentElementButNotChildren() throws Exceptio assertEquals("hello", xmlReader.getValue()); xmlReader.moveDown(); assertEquals("FNARR", xmlReader.getValue()); + xmlReader.moveUp(); + xmlReader.close(); } public void testCanReadLineFeedInString() throws Exception { @@ -241,7 +252,7 @@ public void testCanReadCDATAWithEmbeddedTags() throws Exception { HierarchicalStreamReader xmlReader = createReader(""); assertEquals(content, xmlReader.getValue()); } - + public void testIsXXEVulnerableWithExternalGeneralEntity() throws Exception { HierarchicalStreamReader xmlReader = createReader("" + "\n" @@ -253,7 +264,7 @@ public void testIsXXEVulnerableWithExternalGeneralEntity() throws Exception { +"]>&content;"); assertEquals("", xmlReader.getValue()); } - + public void testIsXXEVulnerableWithExternalParameterEntity() throws Exception { HierarchicalStreamReader xmlReader = createReader("" + "\n" @@ -266,7 +277,37 @@ public void testIsXXEVulnerableWithExternalParameterEntity() throws Exception { +"]>test"); assertEquals("test", xmlReader.getValue()); } - + + public void testCanSkipStructures() throws Exception { + HierarchicalStreamReader xmlReader = createReader("OK"); + xmlReader.moveDown(); + xmlReader.moveDown(); + assertEquals("c", xmlReader.getNodeName()); + assertEquals(3, xmlReader.getLevel()); + + xmlReader.moveUp(); + assertEquals(2, xmlReader.getLevel()); + xmlReader.moveUp(); + assertEquals(1, xmlReader.getLevel()); + + xmlReader.moveDown(); + assertEquals("b2", xmlReader.getNodeName()); + assertEquals(2, xmlReader.getLevel()); + + xmlReader.moveUp(); + assertEquals(1, xmlReader.getLevel()); + + xmlReader.moveDown(); + assertEquals("b3", xmlReader.getNodeName()); + assertEquals(2, xmlReader.getLevel()); + assertEquals("OK", xmlReader.getValue()); + + xmlReader.moveUp(); + assertEquals(1, xmlReader.getLevel()); + + xmlReader.close(); + } + // TODO: See XSTR-473 public void todoTestCanReadNullValueInString() throws Exception { HierarchicalStreamReader xmlReader = createReader("�"); From 20c11e04bf66d348b71d2b1d5422b6e9954a12c9 Mon Sep 17 00:00:00 2001 From: Himanshu Tanwar Date: Thu, 20 Mar 2025 12:11:26 +0530 Subject: [PATCH 2/5] add limits for deserilization --- .../com/thoughtworks/xstream/XStream.java | 109 ++++++++++-------- .../SingleValueConverterWrapper.java | 4 +- .../AbstractReflectionConverter.java | 6 +- .../xstream/core/SecurityUtils.java | 32 ++++- .../acceptance/someobjects/PhoneNumber.java | 19 +++ .../acceptance/someobjects/TestPerson.java | 23 ++++ .../com/thoughtworks/xstream/XStreamTest.java | 107 ++++++++++++++++- 7 files changed, 245 insertions(+), 55 deletions(-) create mode 100644 xstream/src/test/com/thoughtworks/acceptance/someobjects/PhoneNumber.java create mode 100644 xstream/src/test/com/thoughtworks/acceptance/someobjects/TestPerson.java diff --git a/xstream/src/java/com/thoughtworks/xstream/XStream.java b/xstream/src/java/com/thoughtworks/xstream/XStream.java index f0d54f9e8..f6c626fee 100644 --- a/xstream/src/java/com/thoughtworks/xstream/XStream.java +++ b/xstream/src/java/com/thoughtworks/xstream/XStream.java @@ -300,6 +300,10 @@ public class XStream { // self-serialization! private int collectionUpdateLimit = 20; + private int maxAllowedDepth = Integer.MAX_VALUE; + private int maxAllowedFields = Integer.MAX_VALUE; + private int maxAllowedValue = Integer.MAX_VALUE; + private ReflectionProvider reflectionProvider; private HierarchicalStreamDriver hierarchicalStreamDriver; private ClassLoaderReference classLoaderReference; @@ -340,6 +344,10 @@ public class XStream { private static final String ANNOTATION_MAPPER_TYPE = "com.thoughtworks.xstream.mapper.AnnotationMapper"; private static final Pattern IGNORE_ALL = Pattern.compile(".*"); + public static final String MAX_ALLOWED_DEPTH = "XStreamMaxAllowedDepth"; + public static final String MAX_ALLOWED_FIELDS = "XStreamMaxAllowedFields"; + public static final String MAX_ALLOWED_VALUE = "XStreamMaxAllowedValue"; + /** * Constructs a default XStream. *

@@ -358,7 +366,7 @@ public XStream() { *

* The instance will use the {@link XppDriver} as default. *

- * + * * @param reflectionProvider the reflection provider to use or null for best matching reflection provider * @throws InitializationException in case of an initialization problem */ @@ -381,7 +389,7 @@ public XStream(HierarchicalStreamDriver hierarchicalStreamDriver) { /** * Constructs an XStream with a special {@link HierarchicalStreamDriver} and {@link ReflectionProvider}. - * + * * @param reflectionProvider the reflection provider to use or null for best matching Provider * @param hierarchicalStreamDriver the driver instance * @throws InitializationException in case of an initialization problem @@ -393,7 +401,7 @@ public XStream(ReflectionProvider reflectionProvider, HierarchicalStreamDriver h /** * Constructs an XStream with a special {@link HierarchicalStreamDriver}, {@link ReflectionProvider} and a prepared * {@link Mapper} chain. - * + * * @param reflectionProvider the reflection provider to use or null for best matching Provider * @param mapper the instance with the {@link Mapper} chain or null for the default chain * @param driver the driver instance @@ -408,7 +416,7 @@ public XStream(ReflectionProvider reflectionProvider, Mapper mapper, Hierarchica /** * Constructs an XStream with a special {@link HierarchicalStreamDriver}, {@link ReflectionProvider} and a * {@link ClassLoaderReference}. - * + * * @param reflectionProvider the reflection provider to use or null for best matching Provider * @param driver the driver instance * @param classLoaderReference the reference to the {@link ClassLoader} to use @@ -424,7 +432,7 @@ public XStream( /** * Constructs an XStream with a special {@link HierarchicalStreamDriver}, {@link ReflectionProvider} and the * {@link ClassLoader} to use. - * + * * @throws InitializationException in case of an initialization problem * @since 1.3 * @deprecated As of 1.4.5 use {@link #XStream(ReflectionProvider, HierarchicalStreamDriver, ClassLoaderReference)} @@ -436,7 +444,7 @@ public XStream(ReflectionProvider reflectionProvider, HierarchicalStreamDriver d /** * Constructs an XStream with a special {@link HierarchicalStreamDriver}, {@link ReflectionProvider}, a prepared * {@link Mapper} chain and the {@link ClassLoader} to use. - * + * * @param reflectionProvider the reflection provider to use or null for best matching Provider * @param driver the driver instance * @param classLoader the {@link ClassLoader} to use @@ -458,7 +466,7 @@ public XStream( *

* The {@link ClassLoaderReference} should also be used for the {@link Mapper} chain. *

- * + * * @param reflectionProvider the reflection provider to use or null for best matching Provider * @param driver the driver instance * @param classLoaderReference the reference to the {@link ClassLoader} to use @@ -490,7 +498,7 @@ public void registerConverter(Converter converter, int priority) { * Constructs an XStream with a special {@link HierarchicalStreamDriver}, {@link ReflectionProvider}, a prepared * {@link Mapper} chain, the {@link ClassLoaderReference} and an own {@link ConverterLookup} and * {@link ConverterRegistry}. - * + * * @param reflectionProvider the reflection provider to use or null for best matching Provider * @param driver the driver instance * @param classLoader the {@link ClassLoader} to use @@ -518,7 +526,7 @@ public XStream( * ConverterRegistry if you intent to register {@link Converter} instances with XStream facade or you are using * annotations. *

- * + * * @param reflectionProvider the reflection provider to use or null for best matching Provider * @param driver the driver instance * @param classLoaderReference the reference to the {@link ClassLoader} to use @@ -747,7 +755,7 @@ private void denyTypeHierarchyDynamically(String className) { * whitelist of well-known and simply types of the Java runtime as it is done in XStream 1.4.18 by default. This * method will do therefore nothing in XStream 1.4.18 or higher. *

- * + * * @param xstream * @since 1.4.10 * @deprecated As of 1.4.18 @@ -1231,14 +1239,14 @@ public void setMarshallingStrategy(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, if 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 1.4.19 */ @@ -1260,7 +1268,7 @@ public String toXML(Object obj) { /** * Serialize an object to the given Writer as pretty-printed XML. The Writer will be flushed afterwards and in case * of an exception. - * + * * @throws XStreamException if the object cannot be serialized */ public void toXML(Object obj, Writer out) { @@ -1275,7 +1283,7 @@ public void toXML(Object obj, Writer out) { /** * Serialize an object to the given OutputStream as pretty-printed XML. The OutputStream will be flushed afterwards * and in case of an exception. - * + * * @throws XStreamException if the object cannot be serialized */ public void toXML(Object obj, OutputStream out) { @@ -1298,7 +1306,7 @@ public void marshal(Object obj, HierarchicalStreamWriter writer) { /** * Serialize and object to a hierarchical data structure (such as XML). - * + * * @param dataHolder Extra data you can use to pass to your converters. Use this as you want. If not present, * XStream shall create one lazily as needed. * @throws XStreamException if the object cannot be serialized @@ -1337,7 +1345,7 @@ public Object fromXML(InputStream input) { /** * Deserialize an object from a URL. Depending on the parser implementation, some might take the file path as * SystemId to resolve additional references. - * + * * @throws XStreamException if the object cannot be deserialized * @since 1.4 */ @@ -1348,7 +1356,7 @@ public Object fromXML(URL url) { /** * Deserialize an object from a file. Depending on the parser implementation, some might take the file path as * SystemId to resolve additional references. - * + * * @throws XStreamException if the object cannot be deserialized * @since 1.4 */ @@ -1360,7 +1368,7 @@ public Object fromXML(File file) { * Deserialize an object from an XML String, populating the fields of the given root object instead of instantiating * a new one. Note, that this is a special use case! With the ReflectionConverter XStream will write directly into * the raw memory area of the existing object. Use with care! - * + * * @throws XStreamException if the object cannot be deserialized */ public Object fromXML(String xml, Object root) { @@ -1371,7 +1379,7 @@ public Object fromXML(String xml, Object root) { * Deserialize an object from an XML Reader, populating the fields of the given root object instead of instantiating * a new one. Note, that this is a special use case! With the ReflectionConverter XStream will write directly into * the raw memory area of the existing object. Use with care! - * + * * @throws XStreamException if the object cannot be deserialized */ public Object fromXML(Reader xml, Object root) { @@ -1383,7 +1391,7 @@ public Object fromXML(Reader xml, Object root) { * one. Note, that this is a special use case! With the ReflectionConverter XStream will write directly into the raw * memory area of the existing object. Use with care! Depending on the parser implementation, some might take the * file path as SystemId to resolve additional references. - * + * * @throws XStreamException if the object cannot be deserialized * @since 1.4 */ @@ -1401,7 +1409,7 @@ public Object fromXML(URL url, Object root) { * one. Note, that this is a special use case! With the ReflectionConverter XStream will write directly into the raw * memory area of the existing object. Use with care! Depending on the parser implementation, some might take the * file path as SystemId to resolve additional references. - * + * * @throws XStreamException if the object cannot be deserialized * @since 1.4 */ @@ -1418,7 +1426,7 @@ public Object fromXML(File file, Object root) { * Deserialize an object from an XML InputStream, populating the fields of the given root object instead of * instantiating a new one. Note, that this is a special use case! With the ReflectionConverter XStream will write * directly into the raw memory area of the existing object. Use with care! - * + * * @throws XStreamException if the object cannot be deserialized */ public Object fromXML(InputStream input, Object root) { @@ -1438,7 +1446,7 @@ public Object unmarshal(HierarchicalStreamReader reader) { * Deserialize an object from a hierarchical data structure (such as XML), populating the fields of the given root * object instead of instantiating a new one. Note, that this is a special use case! With the ReflectionConverter * XStream will write directly into the raw memory area of the existing object. Use with care! - * + * * @throws XStreamException if the object cannot be deserialized */ public Object unmarshal(HierarchicalStreamReader reader, Object root) { @@ -1447,7 +1455,7 @@ public Object unmarshal(HierarchicalStreamReader reader, Object root) { /** * Deserialize an object from a hierarchical data structure (such as XML). - * + * * @param root If present, the passed in object will have its fields populated, as opposed to XStream creating a new * instance. Note, that this is a special use case! With the ReflectionConverter XStream will write * directly into the raw memory area of the existing object. Use with care! @@ -1465,6 +1473,9 @@ public Object unmarshal(HierarchicalStreamReader reader, Object root, DataHolder dataHolder.put(COLLECTION_UPDATE_SECONDS, new Integer(0)); } try { + dataHolder.put(MAX_ALLOWED_DEPTH, maxAllowedDepth); + dataHolder.put(MAX_ALLOWED_FIELDS, maxAllowedFields); + dataHolder.put(MAX_ALLOWED_VALUE, maxAllowedValue); return marshallingStrategy.unmarshal(root, reader, dataHolder, converterLookup, mapper); } catch (final StackOverflowError e) { throw new InputManipulationException("Possible Denial of Service attack by Stack Overflow"); @@ -1496,7 +1507,7 @@ public void alias(String name, Class type) { /** * Alias a type to a shorter name to be used in XML elements. Any class that is assignable to this type will be * aliased to the same name. - * + * * @param name Short name * @param type Type to be aliased * @since 1.2 @@ -1580,7 +1591,7 @@ public void aliasAttribute(String alias, String attributeName) { * Create an alias for a system attribute. XStream will not write a system attribute if its alias is set to * null. However, this is not reversible, i.e. deserialization of the result is likely to fail * afterwards and will not produce an object equal to the originally written one. - * + * * @param alias the alias itself (may be null) * @param systemAttributeName the name of the system attribute * @throws InitializationException if no {@link SystemAttributeAliasingMapper} is available @@ -1663,7 +1674,7 @@ public void useAttributeFor(Class type) { * Associate a default implementation of a class with an object. Whenever XStream encounters an instance of this * type, it will use the default implementation instead. For example, java.util.ArrayList is the default * implementation of java.util.List. - * + * * @param defaultImplementation * @param ofType * @throws InitializationException if no {@link DefaultImplementationsMapper} is available @@ -1763,7 +1774,7 @@ public void registerLocalConverter(Class definedIn, String fieldName, SingleValu /** * Retrieve the {@link Mapper}. This is by default a chain of {@link MapperWrapper MapperWrappers}. - * + * * @return the mapper * @since 1.2 */ @@ -1789,7 +1800,7 @@ public ConverterLookup getConverterLookup() { * Change mode for dealing with duplicate references. Valid values are XPATH_ABSOLUTE_REFERENCES, * XPATH_RELATIVE_REFERENCES, XStream.ID_REFERENCES and * XStream.NO_REFERENCES. - * + * * @throws IllegalArgumentException if the mode is not one of the declared types * @see #XPATH_ABSOLUTE_REFERENCES * @see #XPATH_RELATIVE_REFERENCES @@ -1851,7 +1862,7 @@ public void addImplicitCollection(Class ownerType, String fieldName, Class itemT /** * Adds implicit collection which is used for all items of the given element name defined by itemFieldName. - * + * * @param ownerType class owning the implicit collection * @param fieldName name of the field in the ownerType. This field must be a concrete collection type or matching * the default implementation type of the collection type. @@ -1876,7 +1887,7 @@ public void addImplicitArray(Class ownerType, String fieldName) { /** * Adds an implicit array which is used for all items of the given itemType when the array type matches. - * + * * @param ownerType class owning the implicit array * @param fieldName name of the array field in the ownerType * @param itemType type of the items to be part of this array @@ -1890,7 +1901,7 @@ public void addImplicitArray(Class ownerType, String fieldName, Class itemType) /** * Adds an implicit array which is used for all items of the given element name defined by itemName. - * + * * @param ownerType class owning the implicit array * @param fieldName name of the array field in the ownerType * @param itemName alias name of the items @@ -1955,7 +1966,7 @@ public DataHolder newDataHolder() { * To change the name of the root element (from <object-stream>), use * {@link #createObjectOutputStream(java.io.Writer, String)}. *

- * + * * @see #createObjectOutputStream(com.thoughtworks.xstream.io.HierarchicalStreamWriter, String) * @see #createObjectInputStream(com.thoughtworks.xstream.io.HierarchicalStreamReader) * @since 1.0.3 @@ -1970,7 +1981,7 @@ public ObjectOutputStream createObjectOutputStream(Writer writer) throws IOExcep * To change the name of the root element (from <object-stream>), use * {@link #createObjectOutputStream(java.io.Writer, String)}. *

- * + * * @see #createObjectOutputStream(com.thoughtworks.xstream.io.HierarchicalStreamWriter, String) * @see #createObjectInputStream(com.thoughtworks.xstream.io.HierarchicalStreamReader) * @since 1.0.3 @@ -1981,7 +1992,7 @@ public ObjectOutputStream createObjectOutputStream(HierarchicalStreamWriter writ /** * Creates an ObjectOutputStream that serializes a stream of objects to the writer using XStream. - * + * * @see #createObjectOutputStream(com.thoughtworks.xstream.io.HierarchicalStreamWriter, String) * @see #createObjectInputStream(com.thoughtworks.xstream.io.HierarchicalStreamReader) * @since 1.0.3 @@ -1996,7 +2007,7 @@ public ObjectOutputStream createObjectOutputStream(Writer writer, String rootNod * To change the name of the root element (from <object-stream>), use * {@link #createObjectOutputStream(java.io.Writer, String)}. *

- * + * * @see #createObjectOutputStream(com.thoughtworks.xstream.io.HierarchicalStreamWriter, String) * @see #createObjectInputStream(com.thoughtworks.xstream.io.HierarchicalStreamReader) * @since 1.3 @@ -2007,7 +2018,7 @@ public ObjectOutputStream createObjectOutputStream(OutputStream out) throws IOEx /** * Creates an ObjectOutputStream that serializes a stream of objects to the OutputStream using XStream. - * + * * @see #createObjectOutputStream(com.thoughtworks.xstream.io.HierarchicalStreamWriter, String) * @see #createObjectInputStream(com.thoughtworks.xstream.io.HierarchicalStreamReader) * @since 1.3 @@ -2085,7 +2096,7 @@ public void close() { /** * Creates an ObjectInputStream that deserializes a stream of objects from a reader using XStream. - * + * * @see #createObjectInputStream(com.thoughtworks.xstream.io.HierarchicalStreamReader) * @see #createObjectOutputStream(com.thoughtworks.xstream.io.HierarchicalStreamWriter, String) * @since 1.0.3 @@ -2096,7 +2107,7 @@ public ObjectInputStream createObjectInputStream(Reader xmlReader) throws IOExce /** * Creates an ObjectInputStream that deserializes a stream of objects from an InputStream using XStream. - * + * * @see #createObjectInputStream(com.thoughtworks.xstream.io.HierarchicalStreamReader) * @see #createObjectOutputStream(com.thoughtworks.xstream.io.HierarchicalStreamWriter, String) * @since 1.3 @@ -2118,7 +2129,7 @@ public ObjectInputStream createObjectInputStream(InputStream in) throws IOExcept * Object b = out.readObject(); * Object c = out.readObject(); * - * + * * @see #createObjectOutputStream(com.thoughtworks.xstream.io.HierarchicalStreamWriter, String) * @since 1.0.3 */ @@ -2177,7 +2188,7 @@ public void close() { * of classes and types of the current JDK, but not for any 3rd party type. To ensure that all other types are * loaded with your class loader, you should call this method as early as possible - or consider to provide the * class loader directly in the constructor. - * + * * @since 1.1.1 */ public void setClassLoader(ClassLoader classLoader) { @@ -2196,7 +2207,7 @@ public ClassLoader getClassLoader() { /** * Retrieve the reference to this instance' ClassLoader. Use this reference for other XStream components (like * converters) to ensure that they will use a changed ClassLoader instance automatically. - * + * * @return the reference * @since 1.4.5 */ @@ -2207,7 +2218,7 @@ public ClassLoaderReference getClassLoaderReference() { /** * Prevents a field from being serialized. To omit a field you must always provide the declaring type and not * necessarily the type that is converted. - * + * * @since 1.1.3 * @throws InitializationException if no {@link ElementIgnoringMapper} is available */ @@ -2270,7 +2281,7 @@ public void processAnnotations(final Class[] types) { /** * Process the annotations of the given type and configure the XStream. A call of this method will automatically * turn the auto-detection mode for annotations off. - * + * * @param type the type with XStream annotations * @since 1.3 */ @@ -2282,7 +2293,7 @@ public void processAnnotations(final Class type) { * Set the auto-detection mode of the AnnotationMapper. Note that auto-detection implies that the XStream is * configured while it is processing the XML steams. This is a potential concurrency problem. Also is it technically * not possible to detect all class aliases at deserialization. You have been warned! - * + * * @param mode true if annotations are auto-detected * @since 1.3 */ @@ -2479,4 +2490,10 @@ public InitializationException(String message) { super(message); } } + + public void setMaxAllowedLimits(int depth, int fields, int value) { + this.maxAllowedDepth = depth; + this.maxAllowedFields = fields; + this.maxAllowedValue = value; + } } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/SingleValueConverterWrapper.java b/xstream/src/java/com/thoughtworks/xstream/converters/SingleValueConverterWrapper.java index 795448df0..e76f0a43b 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/SingleValueConverterWrapper.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/SingleValueConverterWrapper.java @@ -5,11 +5,12 @@ * 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 20. February 2006 by Mauro Talevi */ package com.thoughtworks.xstream.converters; +import com.thoughtworks.xstream.core.SecurityUtils; import com.thoughtworks.xstream.io.HierarchicalStreamReader; import com.thoughtworks.xstream.io.HierarchicalStreamWriter; @@ -46,6 +47,7 @@ public void marshal(Object source, HierarchicalStreamWriter writer, MarshallingC } public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext context) { + SecurityUtils.checkFieldValueLimit(context, reader.getValue()); return fromString(reader.getValue()); } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java index f955b1cb3..4110faf35 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java @@ -18,6 +18,7 @@ import com.thoughtworks.xstream.converters.UnmarshallingContext; import com.thoughtworks.xstream.core.Caching; import com.thoughtworks.xstream.core.ReferencingMarshallingContext; +import com.thoughtworks.xstream.core.SecurityUtils; import com.thoughtworks.xstream.core.util.ArrayIterator; import com.thoughtworks.xstream.core.util.Fields; import com.thoughtworks.xstream.core.util.HierarchicalStreams; @@ -62,7 +63,7 @@ public AbstractReflectionConverter(Mapper mapper, ReflectionProvider reflectionP serializationMethodInvoker = new SerializationMethodInvoker(); serializationMembers = serializationMethodInvoker.serializationMembers; } - + protected boolean canAccess(Class type) { try { reflectionProvider.getFieldOrNull(type, "%"); @@ -283,6 +284,9 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re final Class resultType = result.getClass(); final MemberDictionary seenFields = new MemberDictionary(); + SecurityUtils.checkDepthLimit(context, reader); + SecurityUtils.checkFieldLimit(context, resultType.getDeclaredFields().length); + // process attributes before recursing into child elements. Iterator it = reader.getAttributeNames(); while (it.hasNext()) { diff --git a/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java b/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java index b88cd8feb..a4d769143 100644 --- a/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java +++ b/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java @@ -11,8 +11,10 @@ package com.thoughtworks.xstream.core; import com.thoughtworks.xstream.XStream; +import com.thoughtworks.xstream.XStreamException; import com.thoughtworks.xstream.converters.ConversionException; import com.thoughtworks.xstream.converters.UnmarshallingContext; +import com.thoughtworks.xstream.io.HierarchicalStreamReader; import com.thoughtworks.xstream.security.InputManipulationException; @@ -26,11 +28,11 @@ 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. - * + * 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 1.4.19 @@ -53,4 +55,28 @@ public static void checkForCollectionDoSAttack(final UnmarshallingContext contex } } } + + public static void checkDepthLimit(final UnmarshallingContext context, HierarchicalStreamReader reader) { + Integer maxAllowedDepth = (Integer)context.get(XStream.MAX_ALLOWED_DEPTH); + if(reader.getLevel() > maxAllowedDepth) { + throw new XStreamException("XML depth exceeds maximum allowed depth of " + maxAllowedDepth); + } + if(reader.getNodeName().length() > 5) { + throw new XStreamException("Node name is too long " + reader.getNodeName()); + } + } + + public static void checkFieldLimit(final UnmarshallingContext context, int fieldsLength) { + Integer maxAllowedFields = (Integer)context.get(XStream.MAX_ALLOWED_FIELDS); + if(fieldsLength > maxAllowedFields) { + throw new XStreamException("Encountered more fields than the maximum allowed size of " + maxAllowedFields); + } + } + + public static void checkFieldValueLimit(final UnmarshallingContext context, String value) { + Integer maxAllowedValue = (Integer)context.get(XStream.MAX_ALLOWED_VALUE); + if(value.length() > maxAllowedValue) { + throw new XStreamException("Size of value longer than the maximum allowed size of " + maxAllowedValue); + } + } } diff --git a/xstream/src/test/com/thoughtworks/acceptance/someobjects/PhoneNumber.java b/xstream/src/test/com/thoughtworks/acceptance/someobjects/PhoneNumber.java new file mode 100644 index 000000000..83a5c9bc1 --- /dev/null +++ b/xstream/src/test/com/thoughtworks/acceptance/someobjects/PhoneNumber.java @@ -0,0 +1,19 @@ +package com.thoughtworks.acceptance.someobjects; + +public class PhoneNumber { + private int code; + private String number; + + public PhoneNumber(int code, String number) { + this.code = code; + this.number = number; + } + + @Override + public String toString() { + return "PhoneNumber{" + + "code=" + code + + ", number='" + number + '\'' + + '}'; + } +} diff --git a/xstream/src/test/com/thoughtworks/acceptance/someobjects/TestPerson.java b/xstream/src/test/com/thoughtworks/acceptance/someobjects/TestPerson.java new file mode 100644 index 000000000..e72b0ac82 --- /dev/null +++ b/xstream/src/test/com/thoughtworks/acceptance/someobjects/TestPerson.java @@ -0,0 +1,23 @@ +package com.thoughtworks.acceptance.someobjects; + + +public class TestPerson { + private String firstname; + private String lastname; + private PhoneNumber phone; + + public TestPerson(String firstname, String lastname, PhoneNumber phone) { + this.firstname = firstname; + this.lastname = lastname; + this.phone = phone; + } + + @Override + public String toString() { + return "TestPerson{" + + "firstname='" + firstname + '\'' + + ", lastname='" + lastname + '\'' + + ", phone=" + phone + + '}'; + } +} diff --git a/xstream/src/test/com/thoughtworks/xstream/XStreamTest.java b/xstream/src/test/com/thoughtworks/xstream/XStreamTest.java index c6adf0533..867273db3 100644 --- a/xstream/src/test/com/thoughtworks/xstream/XStreamTest.java +++ b/xstream/src/test/com/thoughtworks/xstream/XStreamTest.java @@ -6,7 +6,7 @@ * 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 26. September 2003 by Joe Walnes */ package com.thoughtworks.xstream; @@ -16,7 +16,9 @@ import com.thoughtworks.acceptance.someobjects.FunnyConstructor; import com.thoughtworks.acceptance.someobjects.Handler; import com.thoughtworks.acceptance.someobjects.HandlerManager; +import com.thoughtworks.acceptance.someobjects.PhoneNumber; import com.thoughtworks.acceptance.someobjects.Protocol; +import com.thoughtworks.acceptance.someobjects.TestPerson; import com.thoughtworks.acceptance.someobjects.U; import com.thoughtworks.acceptance.someobjects.WithList; import com.thoughtworks.acceptance.someobjects.X; @@ -63,6 +65,103 @@ protected void setUp() throws Exception { xstream.alias("with-list", WithList.class); } + public void testUnmarshalObjectsWithDefaultLimits() { + String xml = "\n" + + " John\n" + + " Doe\n" + + " \n" + + " 123\n" + + " 6789\n" + + " \n" + + ""; + + XStream xstreamLimit = new XStream(); + xstreamLimit.allowTypes(new Class[]{TestPerson.class, PhoneNumber.class}); + xstreamLimit.alias("P1", TestPerson.class); + + try { + Object retrievedObject = xstreamLimit.fromXML(xml); + assertEquals(TestPerson.class, retrievedObject.getClass()); + } catch (Exception e) { + + fail("Expected XStream to unmarshall"); + } + } + + public void testUnmarshalObjectsWithDepthLimits() { + String xml = "\n" + + " John\n" + + " Doe\n" + + " \n" + + " 123\n" + + " 6789\n" + + " \n" + + ""; + + XStream xstreamLimit = new XStream(); + xstreamLimit.allowTypes(new Class[]{TestPerson.class, PhoneNumber.class}); + xstreamLimit.alias("P1", TestPerson.class); + + xstreamLimit.setMaxAllowedLimits(1, 5, 5); + try { + xstreamLimit.fromXML(xml); + fail("Expected XStreamException to be thrown"); + } catch (Exception e) { + assertEquals(XStreamException.class, e.getCause().getClass()); + assertEquals("XML depth exceeds maximum allowed depth of 1", e.getCause().getMessage()); + } + } + + public void testUnmarshalObjectsWithFieldsLimits() { + String xml = "\n" + + " John\n" + + " Doe\n" + + " \n" + + " 123\n" + + " 6789\n" + + " \n" + + ""; + + XStream xstreamLimit = new XStream(); + xstreamLimit.allowTypes(new Class[]{TestPerson.class, PhoneNumber.class}); + xstreamLimit.alias("P1", TestPerson.class); + + xstreamLimit.setMaxAllowedLimits(5, 2, 5); + + try { + xstreamLimit.fromXML(xml); + fail("Expected XStreamException to be thrown"); + } catch (Exception e) { + assertEquals(XStreamException.class, e.getCause().getClass()); + assertEquals("Encountered more fields than the maximum allowed size of 2", e.getCause().getMessage()); + } + } + + public void testUnmarshalObjectsWithValueLimits() { + String xml = "\n" + + " John\n" + + " Doe\n" + + " \n" + + " 123\n" + + " 67890\n" + + " \n" + + ""; + + XStream xstreamLimit = new XStream(); + xstreamLimit.allowTypes(new Class[]{TestPerson.class, PhoneNumber.class}); + xstreamLimit.alias("P1", TestPerson.class); + + xstreamLimit.setMaxAllowedLimits(5, 5, 4); + + try { + xstreamLimit.fromXML(xml); + fail("Expected XStreamException to be thrown"); + } catch (Exception e) { + assertEquals(XStreamException.class, e.getCause().getClass()); + assertEquals("Size of value longer than the maximum allowed size of 4", e.getCause().getMessage()); + } + } + public void testUnmarshalsObjectFromXmlWithUnderscores() { String xml = "" + @@ -103,7 +202,7 @@ public void testUnmarshalsObjectFromXmlWithUnderscoresWithoutAliasingFields() { assertEquals("custom value", u.a_Str); } - + public static class U_U { String aStr; } @@ -316,7 +415,7 @@ static class Component { public void testPopulationOfThisAsRootObject() throws Exception { - + String xml ="" + "\n" + " host\n" @@ -334,7 +433,7 @@ public void testPopulationOfThisAsRootObject() assertEquals("host", component.host); assertEquals(8000, component.port); } - + static class SelfSerializingComponent extends Component { String toXML(XStream xstream) { return xstream.toXML(this); From 9294ea92b71928cfec89b9fe1ce6aa1583376bef Mon Sep 17 00:00:00 2001 From: Himanshu Tanwar Date: Mon, 7 Apr 2025 12:18:29 +0530 Subject: [PATCH 3/5] include superclass fields in limit checking --- .../com/thoughtworks/xstream/XStream.java | 6 ++--- .../AbstractReflectionConverter.java | 7 +++-- .../xstream/core/SecurityUtils.java | 3 --- .../acceptance/someobjects/Employee.java | 21 +++++++++++++++ .../com/thoughtworks/xstream/XStreamTest.java | 26 +++++++++++++++++++ 5 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 xstream/src/test/com/thoughtworks/acceptance/someobjects/Employee.java diff --git a/xstream/src/java/com/thoughtworks/xstream/XStream.java b/xstream/src/java/com/thoughtworks/xstream/XStream.java index f6c626fee..f608934c6 100644 --- a/xstream/src/java/com/thoughtworks/xstream/XStream.java +++ b/xstream/src/java/com/thoughtworks/xstream/XStream.java @@ -300,9 +300,9 @@ public class XStream { // self-serialization! private int collectionUpdateLimit = 20; - private int maxAllowedDepth = Integer.MAX_VALUE; - private int maxAllowedFields = Integer.MAX_VALUE; - private int maxAllowedValue = Integer.MAX_VALUE; + private int maxAllowedDepth = 1000; + private int maxAllowedFields = 1000; + private int maxAllowedValue = 50000; private ReflectionProvider reflectionProvider; private HierarchicalStreamDriver hierarchicalStreamDriver; diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java index 4110faf35..9902099c0 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java @@ -284,8 +284,9 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re final Class resultType = result.getClass(); final MemberDictionary seenFields = new MemberDictionary(); - SecurityUtils.checkDepthLimit(context, reader); - SecurityUtils.checkFieldLimit(context, resultType.getDeclaredFields().length); + SecurityUtils.checkDepthLimit(context, reader); + int currentFieldCount = resultType.getDeclaredFields().length; + SecurityUtils.checkFieldLimit(context, currentFieldCount); // process attributes before recursing into child elements. Iterator it = reader.getAttributeNames(); @@ -421,6 +422,8 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re type = mapper.defaultImplementationOf(field.getType()); } // TODO the reflection provider should already return the proper field + currentFieldCount += 1; + SecurityUtils.checkFieldLimit(context, currentFieldCount); value = unmarshallField(context, result, type, field); Class definedType = field.getType(); if (!definedType.isPrimitive()) { diff --git a/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java b/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java index a4d769143..cdbf473ac 100644 --- a/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java +++ b/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java @@ -61,9 +61,6 @@ public static void checkDepthLimit(final UnmarshallingContext context, Hierarchi if(reader.getLevel() > maxAllowedDepth) { throw new XStreamException("XML depth exceeds maximum allowed depth of " + maxAllowedDepth); } - if(reader.getNodeName().length() > 5) { - throw new XStreamException("Node name is too long " + reader.getNodeName()); - } } public static void checkFieldLimit(final UnmarshallingContext context, int fieldsLength) { diff --git a/xstream/src/test/com/thoughtworks/acceptance/someobjects/Employee.java b/xstream/src/test/com/thoughtworks/acceptance/someobjects/Employee.java new file mode 100644 index 000000000..ba516038f --- /dev/null +++ b/xstream/src/test/com/thoughtworks/acceptance/someobjects/Employee.java @@ -0,0 +1,21 @@ +package com.thoughtworks.acceptance.someobjects; + + +public class Employee extends TestPerson { + private String employeeId; + private String department; + + public Employee(String firstname, String lastname, PhoneNumber phone, String employeeId, String department) { + super(firstname, lastname, phone); + this.employeeId = employeeId; + this.department = department; + } + + @Override + public String toString() { + return "Employee{" + + "employeeId='" + employeeId + '\'' + + ", department='" + department + '\'' + + "} " + super.toString(); + } +} diff --git a/xstream/src/test/com/thoughtworks/xstream/XStreamTest.java b/xstream/src/test/com/thoughtworks/xstream/XStreamTest.java index 867273db3..c71b01afd 100644 --- a/xstream/src/test/com/thoughtworks/xstream/XStreamTest.java +++ b/xstream/src/test/com/thoughtworks/xstream/XStreamTest.java @@ -13,6 +13,7 @@ import com.thoughtworks.acceptance.AbstractAcceptanceTest; import com.thoughtworks.acceptance.objects.StandardObject; +import com.thoughtworks.acceptance.someobjects.Employee; import com.thoughtworks.acceptance.someobjects.FunnyConstructor; import com.thoughtworks.acceptance.someobjects.Handler; import com.thoughtworks.acceptance.someobjects.HandlerManager; @@ -137,6 +138,31 @@ public void testUnmarshalObjectsWithFieldsLimits() { } } + public void testUnmarshalObjectsWithFieldsLimitsFromSuperClass() { + String xml = "\n" + + " John\n" + + " Doe\n" + + " \n" + + " 123\n" + + " 6789\n" + + " \n" + + " E123\n" + + " gPQR\n" + + ""; + + XStream xstreamLimit = new XStream(); + xstreamLimit.allowTypes(new Class[]{TestPerson.class, PhoneNumber.class, Employee.class}); + xstreamLimit.setMaxAllowedLimits(5, 4, 5); + + try { + xstreamLimit.fromXML(xml); + fail("Expected XStreamException to be thrown"); + } catch (Exception e) { + assertEquals(XStreamException.class, e.getCause().getClass()); + assertEquals("Encountered more fields than the maximum allowed size of 4", e.getCause().getMessage()); + } + } + public void testUnmarshalObjectsWithValueLimits() { String xml = "\n" + " John\n" + From b6824c869044414b90ee308187797ac147b4259f Mon Sep 17 00:00:00 2001 From: Himanshu Tanwar Date: Tue, 8 Apr 2025 16:03:29 +0530 Subject: [PATCH 4/5] fix duplicate field counting --- .../converters/reflection/AbstractReflectionConverter.java | 3 +-- .../acceptance/MultipleObjectsInOneStreamTest.java | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java index 9902099c0..d2d4ca0d8 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractReflectionConverter.java @@ -285,8 +285,7 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re final MemberDictionary seenFields = new MemberDictionary(); SecurityUtils.checkDepthLimit(context, reader); - int currentFieldCount = resultType.getDeclaredFields().length; - SecurityUtils.checkFieldLimit(context, currentFieldCount); + int currentFieldCount = 0; // process attributes before recursing into child elements. Iterator it = reader.getAttributeNames(); diff --git a/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java b/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java index e642008b0..21deb7839 100644 --- a/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java +++ b/xstream/src/test/com/thoughtworks/acceptance/MultipleObjectsInOneStreamTest.java @@ -234,7 +234,7 @@ public void testFailSafeDeserialization() throws IOException, ClassNotFoundExcep ois.close(); } - public void testFailSafeDeserialization() throws IOException, ClassNotFoundException { + public void testFailSafeDeserializationWithHierarchicalStreamReader() throws IOException, ClassNotFoundException { final String xml = "" + "\n" + " top\n" From 52e4bb4bc9672fe7b29f09b4cae5538d9ec80ec9 Mon Sep 17 00:00:00 2001 From: Himanshu Tanwar Date: Tue, 8 Apr 2025 17:27:14 +0530 Subject: [PATCH 5/5] add null checks --- .../java/com/thoughtworks/xstream/core/SecurityUtils.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java b/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java index cdbf473ac..86e79c2f6 100644 --- a/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java +++ b/xstream/src/java/com/thoughtworks/xstream/core/SecurityUtils.java @@ -58,21 +58,21 @@ public static void checkForCollectionDoSAttack(final UnmarshallingContext contex public static void checkDepthLimit(final UnmarshallingContext context, HierarchicalStreamReader reader) { Integer maxAllowedDepth = (Integer)context.get(XStream.MAX_ALLOWED_DEPTH); - if(reader.getLevel() > maxAllowedDepth) { + if(maxAllowedDepth != null && reader.getLevel() > maxAllowedDepth) { throw new XStreamException("XML depth exceeds maximum allowed depth of " + maxAllowedDepth); } } public static void checkFieldLimit(final UnmarshallingContext context, int fieldsLength) { Integer maxAllowedFields = (Integer)context.get(XStream.MAX_ALLOWED_FIELDS); - if(fieldsLength > maxAllowedFields) { + if(maxAllowedFields != null && fieldsLength > maxAllowedFields) { throw new XStreamException("Encountered more fields than the maximum allowed size of " + maxAllowedFields); } } public static void checkFieldValueLimit(final UnmarshallingContext context, String value) { Integer maxAllowedValue = (Integer)context.get(XStream.MAX_ALLOWED_VALUE); - if(value.length() > maxAllowedValue) { + if(maxAllowedValue != null && value.length() > maxAllowedValue) { throw new XStreamException("Size of value longer than the maximum allowed size of " + maxAllowedValue); } }