Remove "java.lang" package as a default allowed serializable package (#2026) (#2027)

Many classes in the java.lang package should not ever need to be
serialized so this commit removes the default package and instead
includes an allow list of classes that are ok to serialize that are
part of the package. Users have the option to restore the previous
behavior by appending "java.lang" back to the serialized packages
property if desired.

(cherry picked from commit e9ed448)

Author: cshannon

activemq-client/src/main/java/org/apache/activemq/util/ClassLoadingAwareObjectInputStream.java

@@ -32,15 +32,21 @@ public class ClassLoadingAwareObjectInputStream extends ObjectInputStream {
     private static final ClassLoader FALLBACK_CLASS_LOADER =
         ClassLoadingAwareObjectInputStream.class.getClassLoader();
 
+    public static final Set<Class<?>> ALLOWED_JDK_TYPES = Set.of(
+            Boolean.class, Short.class, Integer.class, Long.class,
+            Float.class, Double.class, String.class, Character.class, Byte.class,
+            Throwable.class, Exception.class, StackTraceElement.class);
+
+    public static final String DEFAULT_SERIALIZABLE_PACKAGES = "org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper";
     public static final String[] serializablePackages;
 
-    private List<String> trustedPackages = new ArrayList<String>();
+    private List<String> trustedPackages = new ArrayList<>();
     private boolean trustAllPackages = false;
 
     private final ClassLoader inLoader;
 
     static {
-        serializablePackages = System.getProperty("org.apache.activemq.SERIALIZABLE_PACKAGES","java.lang,org.apache.activemq,org.fusesource.hawtbuf,com.thoughtworks.xstream.mapper").split(",");
+        serializablePackages = System.getProperty("org.apache.activemq.SERIALIZABLE_PACKAGES", DEFAULT_SERIALIZABLE_PACKAGES).split(",");
     }
 
     public ClassLoadingAwareObjectInputStream(InputStream in) throws IOException {
@@ -98,7 +104,7 @@ private boolean trustAllPackages() {
     }
 
     private void checkSecurity(Class clazz) throws ClassNotFoundException {
-        if (trustAllPackages() || clazz.isPrimitive()) {
+        if (trustAllPackages() || clazz.isPrimitive() || ALLOWED_JDK_TYPES.contains(clazz)) {
             return;
         }
 

activemq-client/src/main/java/org/apache/activemq/util/XStreamSupport.java

@@ -28,14 +28,17 @@
 
 public class XStreamSupport {
 
+    private static final Class<?>[] ALLOWED_JDK_TYPES =
+            ClassLoadingAwareObjectInputStream.ALLOWED_JDK_TYPES.toArray(new Class[0]);
+
     public static XStream createXStream() {
         XStream stream = new XStream();
         stream.addPermission(NoTypePermission.NONE);
         stream.addPermission(PrimitiveTypePermission.PRIMITIVES);
         stream.addPermission(ArrayTypePermission.ARRAYS);
         stream.allowTypeHierarchy(Collection.class);
         stream.allowTypeHierarchy(Map.class);
-        stream.allowTypes(new Class[]{String.class});
+        stream.allowTypes(ALLOWED_JDK_TYPES);
         if (ClassLoadingAwareObjectInputStream.isAllAllowed()) {
             stream.addPermission(AnyTypePermission.ANY);
         } else {

activemq-client/src/test/java/org/apache/activemq/util/ClassLoadingAwareObjectInputStreamTest.java

@@ -26,6 +26,7 @@
 import java.io.IOException;
 import java.io.ObjectOutputStream;
 import java.util.Arrays;
+import java.util.Set;
 import java.util.UUID;
 import java.util.Vector;
 
@@ -206,8 +207,23 @@ public void testPrimitveCharNotFiltered() throws Exception {
     }
 
     @Test
-    public void testReadObjectStringNotFiltered() throws Exception {
-        doTestReadObject(new String(name.getMethodName()), ACCEPTS_NONE_FILTER);
+    public void testReadObjectJdkTypesNotFiltered() throws Exception {
+        for (String filter : Set.of(ACCEPTS_ALL_FILTER, ACCEPTS_NONE_FILTER,
+                ClassLoadingAwareObjectInputStream.DEFAULT_SERIALIZABLE_PACKAGES)) {
+            doTestReadObject(Boolean.TRUE, filter);
+            doTestReadObject("test", filter);
+            doTestReadObject(Byte.valueOf("0"), filter);
+            doTestReadObject(Character.valueOf('a'), filter);
+            doTestReadObject(Integer.valueOf(100), filter);
+            doTestReadObject(Long.valueOf(0), filter);
+            doTestReadObject(Float.valueOf(0), filter);
+            doTestReadObject(Double.valueOf(0), filter);
+        }
+
+        // these also require collections classes in java util as well as StackTraceElement
+        // they also can't be compared for equality as they don't implement equals
+        doTestReadObject(new Exception(), "java.util", false);
+        doTestReadObject(new Throwable(), "java.util", false);
     }
 
     //----- Test that primitive arrays get past filters ----------------------//
@@ -429,6 +445,10 @@ public void testReadObjectFailsWithUnstrustedContentInTrustedType() throws Excep
     //----- Internal methods -------------------------------------------------//
 
     private void doTestReadObject(Object value, String filter) throws Exception {
+        doTestReadObject(value, filter, true);
+    }
+
+    private void doTestReadObject(Object value, String filter, boolean equalityCheck) throws Exception {
         byte[] serialized = serializeObject(value);
 
         try (ByteArrayInputStream input = new ByteArrayInputStream(serialized);
@@ -441,10 +461,12 @@ private void doTestReadObject(Object value, String filter) throws Exception {
             Object result = reader.readObject();
             assertNotNull(result);
             assertEquals(value.getClass(), result.getClass());
-            if (result.getClass().isArray()) {
-                assertTrue(Arrays.deepEquals((Object[]) value, (Object[]) result));
-            } else {
-                assertEquals(value, result);
+            if (equalityCheck) {
+                if (result.getClass().isArray()) {
+                    assertTrue(Arrays.deepEquals((Object[]) value, (Object[]) result));
+                } else {
+                    assertEquals(value, result);
+                }
             }
         }
     }

activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompTest.java

@@ -1167,6 +1167,62 @@ public void testTransformationSendJSONObject() throws Exception {
         assertEquals("Dejan", object.getName());
     }
 
+
+    @Test(timeout = 60000)
+    public void testTransformationReceiveXMLObjectDouble() throws Exception {
+        MessageConsumer consumer = session.createConsumer(queue);
+
+        String frame = "CONNECT\n" + "login:system\n" + "passcode:manager\n\n" + Stomp.NULL;
+        stompConnection.sendFrame(frame);
+
+        frame = stompConnection.receiveFrame();
+        assertTrue(frame.startsWith("CONNECTED"));
+
+        // Double should be allowed by default
+        frame = "SEND\n" + "destination:/queue/" + getQueueName() + "\n" +
+                "transformation:" + Stomp.Transformations.JMS_OBJECT_XML + "\n\n" +
+                "<java.lang.Double>1.1</java.lang.Double>" + Stomp.NULL;
+
+        stompConnection.sendFrame(frame);
+
+        Message message = consumer.receive(2500);
+        assertNotNull(message);
+
+        LOG.info("Broker sent: {}", message);
+
+        assertTrue(message instanceof ObjectMessage);
+        ObjectMessage objectMessage = (ObjectMessage)message;
+        Double object = (Double)objectMessage.getObject();
+        assertEquals(Double.valueOf(1.1), object);
+    }
+
+    @Test(timeout = 60000)
+    public void testTransformationSendXMLObjectNotAllowed() throws Exception {
+        MessageConsumer consumer = session.createConsumer(queue);
+
+        String frame = "CONNECT\n" + "login:system\n" + "passcode:manager\n\n" + Stomp.NULL;
+        stompConnection.sendFrame(frame);
+
+        frame = stompConnection.receiveFrame();
+        assertTrue(frame.startsWith("CONNECTED"));
+
+        // ProcessBuilder is not allowed by default so the conversion should fail and
+        // then fall back to using a TextMessage, as well as setting an error header
+        frame = "SEND\n" + "destination:/queue/" + getQueueName() + "\n" +
+                "transformation:" + Stomp.Transformations.JMS_OBJECT_XML + "\n\n" +
+                "<java.lang.ProcessBuilder><command><string>id</string></command></java.lang.ProcessBuilder>" + Stomp.NULL;
+
+        stompConnection.sendFrame(frame);
+
+        Message message = consumer.receive(2500);
+        assertNotNull(message);
+        LOG.info("Broker sent: {}", message);
+
+        // The message should be Text and marked with a transformation error header
+        assertTrue(message instanceof TextMessage);
+        assertEquals("java.lang.ProcessBuilder", message.getStringProperty("transformation-error"));
+    }
+
     @Test(timeout = 60000)
     public void testTransformationSubscribeXML() throws Exception {