Detect input manipulation in c.t.x.io.binary.BinaryStreamReader.

joehni · joehni · commit fdd9f7d3de0d · 2024-11-07T19:48:16.000+01:00
diff --git a/xstream-distribution/src/content/changes.html b/xstream-distribution/src/content/changes.html
@@ -1,7 +1,7 @@
 <html>
 <!--
  Copyright (C) 2005, 2006 Joe Walnes.
- Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023 XStream committers.
+ Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020, 2021, 2022, 2023, 2024 XStream committers.
  All rights reserved.
  
  The software in this package is published under the terms of the BSD
@@ -41,6 +41,7 @@ <h2>Minor changes</h2>
 		<li>GHPR:#334: Fix remaining buffer size calculation in QuickWriter (by Higuchi Yuta).</li>
 		<li>GHI:#342: Optimize internal handling of children in DomReader avoiding O(n²) access times for siblings (by Shiang-Yun Yang).</li>
 		<li>GHI:#359: Add KEYS file with public keys to verify signed artifacts.</li>
+		<li>Detect input manipulation in c.t.x.io.binary.BinaryStreamReader.</li>
 	</ul>
 
 	<h2>API changes</h2>
diff --git a/xstream/src/java/com/thoughtworks/xstream/io/binary/BinaryStreamReader.java b/xstream/src/java/com/thoughtworks/xstream/io/binary/BinaryStreamReader.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2006 Joe Walnes.
- * Copyright (C) 2006, 2007, 2011, 2013 XStream Committers.
+ * Copyright (C) 2006, 2007, 2011, 2013, 2024 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -15,6 +15,7 @@
 import com.thoughtworks.xstream.io.ExtendedHierarchicalStreamReader;
 import com.thoughtworks.xstream.io.HierarchicalStreamReader;
 import com.thoughtworks.xstream.io.StreamException;
+import com.thoughtworks.xstream.security.InputManipulationException;
 
 import java.io.DataInputStream;
 import java.io.IOException;
@@ -150,15 +151,20 @@ public void moveUp() {
     private Token readToken() {
         if (pushback == null) {
             try {
-                Token token = tokenFormatter.read(in);
-                switch (token.getType()) {
+                boolean mapping = false;
+                do {
+                    final Token token = tokenFormatter.read(in);
+                    switch (token.getType()) {
                     case Token.TYPE_MAP_ID_TO_VALUE:
                         idRegistry.put(token.getId(), token.getValue());
-                        return readToken(); // Next one please.
+                        mapping ^= true;
+                        continue; // Next one please.
                     default:
                         return token;
-                }
-            } catch (IOException e) {
+                    }
+                } while (mapping);
+                throw new InputManipulationException("Binary stream will never have two mapping tokens in sequence");
+            } catch (final IOException e) {
                 throw new StreamException(e);
             }
         } else {
diff --git a/xstream/src/test/com/thoughtworks/xstream/io/binary/BinaryStreamTest.java b/xstream/src/test/com/thoughtworks/xstream/io/binary/BinaryStreamTest.java
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2006 Joe Walnes.
- * Copyright (C) 2006, 2007, 2011, 2015, 2016, 2021 XStream Committers.
+ * Copyright (C) 2006, 2007, 2011, 2015, 2016, 2021, 2024 XStream Committers.
  * All rights reserved.
  *
  * The software in this package is published under the terms of the BSD
@@ -17,10 +17,12 @@
 import com.thoughtworks.xstream.io.copy.HierarchicalStreamCopier;
 import com.thoughtworks.xstream.io.xml.AbstractXMLReaderTest;
 import com.thoughtworks.xstream.io.xml.MXParserDriver;
+import com.thoughtworks.xstream.security.InputManipulationException;
 
 import java.io.ByteArrayOutputStream;
 import java.io.StringReader;
 import java.io.ByteArrayInputStream;
+import java.io.InputStream;
 
 public class BinaryStreamTest extends AbstractXMLReaderTest {
 
@@ -89,4 +91,17 @@ public void testIsXXEVulnerableWithExternalGeneralEntity() throws Exception {
         }
     }
 
+    public void testHandleMaliciousInputsOfIdMappingTokens() {
+        // Insert two successive id mapping tokens into the stream
+        final byte[] byteArray = new byte[8];
+        byteArray[0] = byteArray[4] = 10;
+        byteArray[1] = byteArray[5] = -127;
+
+        final InputStream in = new ByteArrayInputStream(byteArray);
+        try {
+            new BinaryStreamReader(in);
+            fail("Thrown " + InputManipulationException.class.getName() + " expected");
+        } catch (final InputManipulationException e) {
+        }
+    }
 }
