This is an automated email from the ASF dual-hosted git repository. pkarwasz pushed a commit to branch fix/3680_serialize_arrays in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit a82345d90d94ecb70404c41d12e7e808ffca018d Author: Piotr P. Karwasz <[email protected]> AuthorDate: Fri Feb 2 10:27:46 2024 +0100 [LOG4J2-3680] Allow deserialization of arrays The `DefaultObjectInputFilter` is too restrictive and it does not allow the deserialization of **arrays** of classes in the `SerializationUtil#REQUIRED_JAVA_CLASSES` and `SerializationUtil#REQUIRED_JAVA_PACKAGES` white lists. Most notably `RuntimeException` can not be deserialized, since it contains an array of `StackTraceElement`s. Both classes are allowed, but deserialization of the array fails. --- .../util/internal/DefaultObjectInputFilter.java | 9 +-- .../log4j/util/internal/SerializationUtil.java | 4 ++ .../logging/log4j/test/junit/SerialUtil.java | 49 +++++++++++++--- .../logging/log4j/test/junit/package-info.java | 2 +- .../logging/log4j/message/ObjectMessageTest.java | 8 ++- .../log4j/message/ParameterizedMessageTest.java | 16 ++++- .../log4j/util/SortedArrayStringMapTest.java | 21 +------ .../log4j/util/internal/SerializationUtilTest.java | 54 +++++++++++++++++ .../log4j/util/FilteredObjectInputStream.java | 3 +- .../log4j/util/internal/SerializationUtil.java | 68 ++++++++++++++++++++-- .../apache/logging/log4j/core/LogEventTest.java | 60 +++++-------------- .../core/appender/mom/kafka/KafkaAppenderTest.java | 16 ++--- .../log4j/core/async/RingBufferLogEventTest.java | 13 +---- .../logging/log4j/core/impl/Log4jLogEventTest.java | 22 +------ .../log4j/core/impl/MutableLogEventTest.java | 36 +----------- .../log4j/core/layout/SerializedLayoutTest.java | 33 +++-------- .../LOG4J2-3680_fix_serial_filter_arrays.xml | 10 ++++ .../2.22.0/1906_harden_serialization_process.xml | 10 ++++ 18 files changed, 246 insertions(+), 188 deletions(-) diff --git a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/DefaultObjectInputFilter.java b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/DefaultObjectInputFilter.java index 1c45654168..df62f71e5f 100644 --- a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/DefaultObjectInputFilter.java +++ b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/DefaultObjectInputFilter.java @@ -44,7 +44,7 @@ public class DefaultObjectInputFilter implements ObjectInputFilter { @Override public Status checkInput(final FilterInfo filterInfo) { - Status status = null; + Status status; if (delegate != null) { status = delegate.checkInput(filterInfo); if (status != Status.UNDECIDED) { @@ -59,9 +59,10 @@ public class DefaultObjectInputFilter implements ObjectInputFilter { return status; } } - if (filterInfo.serialClass() != null) { - final String name = filterInfo.serialClass().getName(); - if (isAllowedByDefault(name) || isRequiredPackage(name)) { + final Class<?> serialClass = filterInfo.serialClass(); + if (serialClass != null) { + final String name = SerializationUtil.stripArray(serialClass); + if (isAllowedByDefault(name)) { return Status.ALLOWED; } } else { diff --git a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java index bd4ab1ad80..ddade3dd8a 100644 --- a/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java +++ b/log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java @@ -24,4 +24,8 @@ import java.util.List; public final class SerializationUtil { public static final List<String> REQUIRED_JAVA_CLASSES = List.of(); public static final List<String> REQUIRED_JAVA_PACKAGES = List.of(); + + public static String stripArray(final Class<?> clazz) { + return null; + } } diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/SerialUtil.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/SerialUtil.java index 943fb39b0f..e568e41513 100644 --- a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/SerialUtil.java +++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/SerialUtil.java @@ -19,7 +19,10 @@ package org.apache.logging.log4j.test.junit; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; import java.io.ObjectInputStream; +import java.io.ObjectOutput; import java.io.ObjectOutputStream; import java.io.Serializable; import org.apache.logging.log4j.util.Constants; @@ -38,10 +41,21 @@ public class SerialUtil { * @return the serialized object */ public static byte[] serialize(final Serializable obj) { + return serialize(new Serializable[] {obj}); + } + + /** + * Serializes the specified object and returns the result as a byte array. + * @param objs an array of objects to serialize + * @return the serialized object + */ + public static byte[] serialize(final Serializable... objs) { try { final ByteArrayOutputStream bas = new ByteArrayOutputStream(8192); - final ObjectOutputStream oos = new ObjectOutputStream(bas); - oos.writeObject(obj); + final ObjectOutput oos = new ObjectOutputStream(bas); + for (final Object obj : objs) { + oos.writeObject(obj); + } oos.flush(); return bas.toByteArray(); } catch (final Exception ex) { @@ -58,16 +72,33 @@ public class SerialUtil { @SuppressFBWarnings("OBJECT_DESERIALIZATION") public static <T> T deserialize(final byte[] data) { try { - final ByteArrayInputStream bas = new ByteArrayInputStream(data); - final ObjectInputStream ois; - if (Constants.JAVA_MAJOR_VERSION == 8) { - ois = new FilteredObjectInputStream(bas); - } else { - ois = new ObjectInputStream(bas); - } + final ObjectInputStream ois = getObjectInputStream(data); return (T) ois.readObject(); } catch (final Exception ex) { throw new IllegalStateException("Could not deserialize", ex); } } + + /** + * Creates an {@link ObjectInputStream} adapted to the current Java version. + * @param data data to deserialize, + * @return an object input stream. + */ + @SuppressFBWarnings("OBJECT_DESERIALIZATION") + public static ObjectInputStream getObjectInputStream(final byte[] data) throws IOException { + final ByteArrayInputStream bas = new ByteArrayInputStream(data); + return getObjectInputStream(bas); + } + + /** + * Creates an {@link ObjectInputStream} adapted to the current Java version. + * @param stream stream of data to deserialize, + * @return an object input stream. + */ + @SuppressFBWarnings("OBJECT_DESERIALIZATION") + public static ObjectInputStream getObjectInputStream(final InputStream stream) throws IOException { + return Constants.JAVA_MAJOR_VERSION == 8 + ? new FilteredObjectInputStream(stream) + : new ObjectInputStream(stream); + } } diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java index 5b84db02c2..4ead204b3b 100644 --- a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java +++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java @@ -15,7 +15,7 @@ * limitations under the license. */ @Export -@Version("2.21.1") +@Version("2.23.0") package org.apache.logging.log4j.test.junit; import org.osgi.annotation.bundle.Export; diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java index 117cd47315..b2ef7ca978 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java @@ -92,7 +92,13 @@ public class ObjectMessageTest { return other instanceof NonSerializable; // a very lenient equals() } } - return Stream.of("World", new NonSerializable(), new BigDecimal("123.456"), null); + return Stream.of( + "World", + new NonSerializable(), + new BigDecimal("123.456"), + // LOG4J2-3680 + new RuntimeException(), + null); } @ParameterizedTest diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java index 65ff5d9a96..1aad0d64d9 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java @@ -19,6 +19,7 @@ package org.apache.logging.log4j.message; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import java.math.BigDecimal; import java.util.stream.Stream; import org.apache.logging.log4j.test.junit.Mutable; import org.apache.logging.log4j.test.junit.SerialUtil; @@ -149,7 +150,20 @@ public class ParameterizedMessageTest { } static Stream<Object> testSerializable() { - return Stream.of("World", new Object(), null); + @SuppressWarnings("EqualsHashCode") + class NonSerializable { + @Override + public boolean equals(final Object other) { + return other instanceof NonSerializable; // a very lenient equals() + } + } + return Stream.of( + "World", + new NonSerializable(), + new BigDecimal("123.456"), + // LOG4J2-3680 + new RuntimeException(), + null); } @ParameterizedTest diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/SortedArrayStringMapTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/SortedArrayStringMapTest.java index 4904f31341..164ae85f3f 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/SortedArrayStringMapTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/SortedArrayStringMapTest.java @@ -16,6 +16,8 @@ */ package org.apache.logging.log4j.util; +import static org.apache.logging.log4j.test.junit.SerialUtil.deserialize; +import static org.apache.logging.log4j.test.junit.SerialUtil.serialize; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -27,14 +29,9 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.fail; import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileOutputStream; -import java.io.IOException; import java.io.InputStreamReader; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.lang.reflect.Field; import java.net.URL; import java.net.URLDecoder; @@ -191,20 +188,6 @@ public class SortedArrayStringMapTest { return location.isEmpty() ? "." : location; } - private byte[] serialize(final SortedArrayStringMap data) throws IOException { - final ByteArrayOutputStream arr = new ByteArrayOutputStream(); - final ObjectOutputStream out = new ObjectOutputStream(arr); - out.writeObject(data); - return arr.toByteArray(); - } - - private SortedArrayStringMap deserialize(final byte[] binary) throws IOException, ClassNotFoundException { - final ByteArrayInputStream inArr = new ByteArrayInputStream(binary); - try (final ObjectInputStream in = new FilteredObjectInputStream(inArr)) { - return (SortedArrayStringMap) in.readObject(); - } - } - @Test public void testPutAll() { final SortedArrayStringMap original = new SortedArrayStringMap(); diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/internal/SerializationUtilTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/internal/SerializationUtilTest.java new file mode 100644 index 0000000000..8b1d5d9dbb --- /dev/null +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/internal/SerializationUtilTest.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.logging.log4j.util.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +class SerializationUtilTest { + + static Stream<Arguments> arrays() { + return Stream.of( + Arguments.of(boolean[].class, boolean.class), + Arguments.of(char[].class, char.class), + Arguments.of(byte[].class, byte.class), + Arguments.of(short[].class, short.class), + Arguments.of(int[].class, int.class), + Arguments.of(long[].class, long.class), + Arguments.of(float[].class, float.class), + Arguments.of(double[].class, double.class), + Arguments.of(String.class, String.class), + Arguments.of(String[].class, String.class), + Arguments.of(String[][].class, String.class)); + } + + @ParameterizedTest + @MethodSource("arrays") + void stripArrayClass(final Class<?> arrayClass, final Class<?> componentClazz) { + assertThat(SerializationUtil.stripArray(arrayClass)).isEqualTo(componentClazz.getName()); + } + + @ParameterizedTest + @MethodSource("arrays") + void stripArrayString(final Class<?> arrayClass, final Class<?> componentClazz) { + assertThat(SerializationUtil.stripArray(arrayClass.getName())).isEqualTo(componentClazz.getName()); + } +} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/FilteredObjectInputStream.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/FilteredObjectInputStream.java index f7b139a110..52b71507de 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/FilteredObjectInputStream.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/FilteredObjectInputStream.java @@ -26,6 +26,7 @@ import java.io.ObjectInputStream; import java.io.ObjectStreamClass; import java.util.Collection; import java.util.Collections; +import org.apache.logging.log4j.util.internal.SerializationUtil; /** * Extends {@link ObjectInputStream} to only allow some built-in Log4j classes and caller-specified classes to be @@ -63,7 +64,7 @@ public class FilteredObjectInputStream extends ObjectInputStream { @Override protected Class<?> resolveClass(final ObjectStreamClass desc) throws IOException, ClassNotFoundException { - final String name = desc.getName(); + final String name = SerializationUtil.stripArray(desc.getName()); if (!(isAllowedByDefault(name) || allowedExtraClasses.contains(name))) { throw new InvalidObjectException("Class is not allowed for deserialization: " + name); } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java index 95899dbe8a..de986b8231 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java @@ -79,12 +79,18 @@ public final class SerializationUtil { "java.math.BigInteger", // for Message delegate "java.rmi.MarshalledObject", - "[B", - // for MessagePatternAnalysis - "[I"); + // all primitives + "boolean", + "byte", + "char", + "double", + "float", + "int", + "long", + "short"); - public static final List<String> REQUIRED_JAVA_PACKAGES = Arrays.asList( - "java.lang.", "java.time", "java.util.", "org.apache.logging.log4j.", "[Lorg.apache.logging.log4j."); + public static final List<String> REQUIRED_JAVA_PACKAGES = + Arrays.asList("java.lang.", "java.time.", "java.util.", "org.apache.logging.log4j."); public static void writeWrappedObject(final Serializable obj, final ObjectOutputStream out) throws IOException { final ByteArrayOutputStream bout = new ByteArrayOutputStream(); @@ -133,5 +139,57 @@ public final class SerializationUtil { } } + /** + * Gets the class name of an array component. + * <p> + * If {@code clazz} is not an array class its name is returned. + * </p> + * @param clazz the binary name of a class. + */ + public static String stripArray(final Class<?> clazz) { + return clazz.isArray() ? stripArray(clazz.getComponentType()) : clazz.getName(); + } + + /** + * Gets the class name of an array component. + * <p> + * If {@code name} is not the name of an array class it is returned unchanged. + * </p> + * @param name the binary name of a class. + * @see Class#getName() + */ + public static String stripArray(final String name) { + int offset; + for (offset = 0; offset < name.length() && name.charAt(offset) == '['; offset++) {} + ; + if (offset == 0) { + return name; + } + if (name.charAt(offset) == 'L') { + return name.substring(offset + 1, name.length() - 1); + } + // Primitive classes + switch (name.substring(offset)) { + case "Z": + return "boolean"; + case "B": + return "byte"; + case "C": + return "char"; + case "D": + return "double"; + case "F": + return "float"; + case "I": + return "int"; + case "J": + return "long"; + case "S": + return "short"; + default: + return name; + } + } + private SerializationUtil() {} } diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LogEventTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LogEventTest.java index 525578812a..ec462b1f4d 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LogEventTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LogEventTest.java @@ -16,22 +16,18 @@ */ package org.apache.logging.log4j.core; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.fail; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.LoggingException; import org.apache.logging.log4j.core.impl.Log4jLogEvent; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.SimpleMessage; -import org.apache.logging.log4j.util.FilteredObjectInputStream; +import org.apache.logging.log4j.test.junit.SerialUtil; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; @@ -61,28 +57,17 @@ public class LogEventTest { .setThrown(child) // .build(); - final ByteArrayOutputStream baos = new ByteArrayOutputStream(); - final ObjectOutputStream oos = new ObjectOutputStream(baos); - oos.writeObject(event1); - oos.writeObject(event2); - - final ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); - final ObjectInputStream ois = new FilteredObjectInputStream(bais); - try { - ois.readObject(); - } catch (final IOException ioe) { - fail("Exception processing event1"); - } - try { - ois.readObject(); - } catch (final IOException ioe) { - fail("Exception processing event2"); + final byte[] data = SerialUtil.serialize(event1, event2); + + try (final ObjectInputStream ois = SerialUtil.getObjectInputStream(data)) { + assertDoesNotThrow(ois::readObject, "Failed to deserialize event1"); + assertDoesNotThrow(ois::readObject, "Failed to deserialize event1"); } } @Test public void testNanoTimeIsNotSerialized1() throws Exception { - final LogEvent event1 = Log4jLogEvent.newBuilder() // + final LogEvent event = Log4jLogEvent.newBuilder() // .setLoggerName(this.getClass().getName()) // .setLoggerFqcn("org.apache.logging.log4j.core.Logger") // .setLevel(Level.INFO) // @@ -90,24 +75,18 @@ public class LogEventTest { .setThreadName("this must be initialized or the test fails") // .setNanoTime(12345678L) // .build(); - final LogEvent copy = new Log4jLogEvent.Builder(event1).build(); - - final ByteArrayOutputStream baos = new ByteArrayOutputStream(); - final ObjectOutputStream oos = new ObjectOutputStream(baos); - oos.writeObject(event1); - final ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); - final ObjectInputStream ois = new FilteredObjectInputStream(bais); + final LogEvent expected = new Log4jLogEvent.Builder(event).build(); + final LogEvent actual = SerialUtil.deserialize(SerialUtil.serialize(event)); - final LogEvent actual = (LogEvent) ois.readObject(); - assertNotEquals(copy, actual, "Different event: nanoTime"); - assertNotEquals(copy.getNanoTime(), actual.getNanoTime(), "Different nanoTime"); + assertNotEquals(expected, actual, "Different event: nanoTime"); + assertNotEquals(expected.getNanoTime(), actual.getNanoTime(), "Different nanoTime"); assertEquals(0, actual.getNanoTime(), "deserialized nanoTime is zero"); } @Test public void testNanoTimeIsNotSerialized2() throws Exception { - final LogEvent event1 = Log4jLogEvent.newBuilder() // + final LogEvent event = Log4jLogEvent.newBuilder() // .setLoggerName(this.getClass().getName()) // .setLoggerFqcn("org.apache.logging.log4j.core.Logger") // .setLevel(Level.INFO) // @@ -117,17 +96,10 @@ public class LogEventTest { .setThreadPriority(2) // this must be initialized or the test fails .setNanoTime(0) // .build(); - final LogEvent event2 = new Log4jLogEvent.Builder(event1).build(); - - final ByteArrayOutputStream baos = new ByteArrayOutputStream(); - final ObjectOutputStream oos = new ObjectOutputStream(baos); - oos.writeObject(event1); - - final ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray()); - final ObjectInputStream ois = new FilteredObjectInputStream(bais); - final LogEvent actual = (LogEvent) ois.readObject(); - assertEquals(event2, actual, "both zero nanoTime"); + final LogEvent expected = new Log4jLogEvent.Builder(event).build(); + final LogEvent actual = SerialUtil.deserialize(SerialUtil.serialize(event)); + assertEquals(expected, actual, "both zero nanoTime"); } @Test diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/mom/kafka/KafkaAppenderTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/mom/kafka/KafkaAppenderTest.java index 8df1ce062c..ac69d2e944 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/mom/kafka/KafkaAppenderTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/mom/kafka/KafkaAppenderTest.java @@ -22,9 +22,6 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import java.io.ByteArrayInputStream; -import java.io.IOException; -import java.io.ObjectInput; import java.nio.charset.StandardCharsets; import java.text.SimpleDateFormat; import java.time.Duration; @@ -46,7 +43,7 @@ import org.apache.logging.log4j.core.impl.Log4jLogEvent; import org.apache.logging.log4j.core.test.categories.Appenders; import org.apache.logging.log4j.core.test.junit.LoggerContextRule; import org.apache.logging.log4j.message.SimpleMessage; -import org.apache.logging.log4j.util.FilteredObjectInputStream; +import org.apache.logging.log4j.test.junit.SerialUtil; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; @@ -141,7 +138,9 @@ public class KafkaAppenderTest { assertNotNull(item); assertEquals(TOPIC_NAME, item.topic()); assertNull(item.key()); - assertEquals(LOG_MESSAGE, deserializeLogEvent(item.value()).getMessage().getFormattedMessage()); + final byte[] data = item.value(); + assertEquals( + LOG_MESSAGE, SerialUtil.<LogEvent>deserialize(data).getMessage().getFormattedMessage()); } @Test @@ -224,13 +223,6 @@ public class KafkaAppenderTest { assertEquals(LOG_MESSAGE, new String(item.value(), StandardCharsets.UTF_8)); } - private LogEvent deserializeLogEvent(final byte[] data) throws IOException, ClassNotFoundException { - final ByteArrayInputStream bis = new ByteArrayInputStream(data); - try (final ObjectInput ois = new FilteredObjectInputStream(bis)) { - return (LogEvent) ois.readObject(); - } - } - // public void shouldRetryWhenTimeoutExceptionOccursOnSend() throws Exception { // final AtomicInteger attempt = new AtomicInteger(0); // final RecordCollectorImpl collector = new RecordCollectorImpl( diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/RingBufferLogEventTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/RingBufferLogEventTest.java index a1ae1efe31..ffd26bed64 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/RingBufferLogEventTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/RingBufferLogEventTest.java @@ -24,11 +24,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.util.Arrays; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; @@ -44,7 +40,7 @@ import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.ReusableMessageFactory; import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.spi.MutableThreadContextStack; -import org.apache.logging.log4j.util.FilteredObjectInputStream; +import org.apache.logging.log4j.test.junit.SerialUtil; import org.apache.logging.log4j.util.StringMap; import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.jupiter.api.Tag; @@ -227,12 +223,7 @@ class RingBufferLogEventTest { new DummyNanoClock(1)); ((StringMap) evt.getContextData()).putValue("key", "value"); - final ByteArrayOutputStream baos = new ByteArrayOutputStream(); - final ObjectOutputStream out = new ObjectOutputStream(baos); - out.writeObject(evt); - - final ObjectInputStream in = new FilteredObjectInputStream(new ByteArrayInputStream(baos.toByteArray())); - final RingBufferLogEvent other = (RingBufferLogEvent) in.readObject(); + final RingBufferLogEvent other = SerialUtil.deserialize(SerialUtil.serialize(evt)); assertThat(other.getLoggerName()).isEqualTo(loggerName); assertThat(other.getMarker()).isEqualTo(marker); assertThat(other.getLoggerFqcn()).isEqualTo(fqcn); diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java index 3cfd9235d1..119e3dd1cc 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/Log4jLogEventTest.java @@ -16,6 +16,8 @@ */ package org.apache.logging.log4j.core.impl; +import static org.apache.logging.log4j.test.junit.SerialUtil.deserialize; +import static org.apache.logging.log4j.test.junit.SerialUtil.serialize; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; @@ -25,11 +27,6 @@ import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.lang.reflect.Field; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Marker; @@ -47,7 +44,6 @@ import org.apache.logging.log4j.message.ObjectMessage; import org.apache.logging.log4j.message.ReusableMessage; import org.apache.logging.log4j.message.ReusableObjectMessage; import org.apache.logging.log4j.message.SimpleMessage; -import org.apache.logging.log4j.util.FilteredObjectInputStream; import org.apache.logging.log4j.util.SortedArrayStringMap; import org.apache.logging.log4j.util.StringMap; import org.apache.logging.log4j.util.Strings; @@ -158,20 +154,6 @@ public class Log4jLogEventTest { assertEquals(evt.isIncludeLocation(), evt2.isIncludeLocation()); } - private byte[] serialize(final Log4jLogEvent event) throws IOException { - final ByteArrayOutputStream arr = new ByteArrayOutputStream(); - final ObjectOutputStream out = new ObjectOutputStream(arr); - out.writeObject(event); - return arr.toByteArray(); - } - - private Log4jLogEvent deserialize(final byte[] binary) throws IOException, ClassNotFoundException { - final ByteArrayInputStream inArr = new ByteArrayInputStream(binary); - final ObjectInputStream in = new FilteredObjectInputStream(inArr); - final Log4jLogEvent result = (Log4jLogEvent) in.readObject(); - return result; - } - // DO NOT REMOVE THIS COMMENT: // UNCOMMENT WHEN GENERATING SERIALIZED EVENT FOR #testJavaIoSerializableWithUnknownThrowable // public static class DeletedException extends Exception { diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/MutableLogEventTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/MutableLogEventTest.java index ce7b1c543b..0723d6ff73 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/MutableLogEventTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/MutableLogEventTest.java @@ -16,6 +16,8 @@ */ package org.apache.logging.log4j.core.impl; +import static org.apache.logging.log4j.test.junit.SerialUtil.deserialize; +import static org.apache.logging.log4j.test.junit.SerialUtil.serialize; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -27,11 +29,6 @@ import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertTrue; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.ObjectInputStream; -import java.io.ObjectOutputStream; import java.util.Arrays; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.MarkerManager; @@ -43,10 +40,8 @@ import org.apache.logging.log4j.message.ReusableMessageFactory; import org.apache.logging.log4j.message.ReusableSimpleMessage; import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.spi.MutableThreadContextStack; -import org.apache.logging.log4j.util.FilteredObjectInputStream; import org.apache.logging.log4j.util.SortedArrayStringMap; import org.apache.logging.log4j.util.StringMap; -import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; /** @@ -56,8 +51,6 @@ public class MutableLogEventTest { private static final StringMap CONTEXT_DATA = createContextData(); private static final ThreadContext.ContextStack STACK = new MutableThreadContextStack(Arrays.asList("abc", "xyz")); - static boolean useObjectInputStream = false; - private static StringMap createContextData() { final StringMap result = new SortedArrayStringMap(); result.putValue("a", "1"); @@ -65,16 +58,6 @@ public class MutableLogEventTest { return result; } - @BeforeAll - public static void setupClass() { - try { - Class.forName("java.io.ObjectInputFilter"); - useObjectInputStream = true; - } catch (final ClassNotFoundException ex) { - // Ignore the exception - } - } - @Test public void testToImmutable() { final LogEvent logEvent = new MutableLogEvent(); @@ -383,19 +366,4 @@ public class MutableLogEventTest { final Log4jLogEvent immutable = mutable.toImmutable(); assertThat(immutable.getSource()).isEqualTo(source); } - - private byte[] serialize(final MutableLogEvent event) throws IOException { - final ByteArrayOutputStream arr = new ByteArrayOutputStream(); - final ObjectOutputStream out = new ObjectOutputStream(arr); - out.writeObject(event); - return arr.toByteArray(); - } - - private Log4jLogEvent deserialize(final byte[] binary) throws IOException, ClassNotFoundException { - final ByteArrayInputStream inArr = new ByteArrayInputStream(binary); - final ObjectInputStream in = - useObjectInputStream ? new ObjectInputStream(inArr) : new FilteredObjectInputStream(inArr); - final Log4jLogEvent result = (Log4jLogEvent) in.readObject(); - return result; - } } diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/SerializedLayoutTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/SerializedLayoutTest.java index 1c7c03ac62..7f1fb2e849 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/SerializedLayoutTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/layout/SerializedLayoutTest.java @@ -16,14 +16,13 @@ */ package org.apache.logging.log4j.core.layout; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileOutputStream; -import java.io.IOException; import java.io.ObjectInputStream; import java.util.List; import java.util.Map; @@ -39,8 +38,8 @@ import org.apache.logging.log4j.core.impl.Log4jLogEvent; import org.apache.logging.log4j.core.test.BasicConfigurationFactory; import org.apache.logging.log4j.core.test.appender.ListAppender; import org.apache.logging.log4j.message.SimpleMessage; +import org.apache.logging.log4j.test.junit.SerialUtil; import org.apache.logging.log4j.test.junit.ThreadContextRule; -import org.apache.logging.log4j.util.FilteredObjectInputStream; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Rule; @@ -56,19 +55,11 @@ public class SerializedLayoutTest { static ConfigurationFactory cf = new BasicConfigurationFactory(); - static boolean useObjectInputStream = false; - @Rule public final ThreadContextRule threadContextRule = new ThreadContextRule(); @BeforeClass public static void setupClass() { - try { - Class.forName("java.io.ObjectInputFilter"); - useObjectInputStream = true; - } catch (final ClassNotFoundException ex) { - // Ignore the exception - } ConfigurationFactory.setConfigurationFactory(cf); final LoggerContext ctx = LoggerContext.getContext(); ctx.reconfigure(); @@ -130,20 +121,11 @@ public class SerializedLayoutTest { appender.stop(); final List<byte[]> data = appender.getData(); - assertTrue(data.size() > 0); + assertFalse(data.isEmpty()); int i = 0; for (final byte[] item : data) { - final ByteArrayInputStream bais = new ByteArrayInputStream(item); - final ObjectInputStream ois = - useObjectInputStream ? new ObjectInputStream(bais) : new FilteredObjectInputStream(bais); - LogEvent event; - try { - event = (LogEvent) ois.readObject(); - } catch (final IOException ioe) { - System.err.println("Exception processing item " + i); - throw ioe; - } - assertTrue("Incorrect event", event.toString().equals(expected[i])); + assertEquals( + "Incorrect event", expected[i], SerialUtil.deserialize(item).toString()); ++i; } for (final Appender app : appenders.values()) { @@ -175,8 +157,7 @@ public class SerializedLayoutTest { testSerialization(); final File file = new File(DAT_PATH); final FileInputStream fis = new FileInputStream(file); - try (final ObjectInputStream ois = - useObjectInputStream ? new ObjectInputStream(fis) : new FilteredObjectInputStream(fis)) { + try (final ObjectInputStream ois = SerialUtil.getObjectInputStream(fis)) { final LogEvent event = (LogEvent) ois.readObject(); assertNotNull(event); } diff --git a/src/changelog/.2.x.x/LOG4J2-3680_fix_serial_filter_arrays.xml b/src/changelog/.2.x.x/LOG4J2-3680_fix_serial_filter_arrays.xml new file mode 100644 index 0000000000..a8b871472f --- /dev/null +++ b/src/changelog/.2.x.x/LOG4J2-3680_fix_serial_filter_arrays.xml @@ -0,0 +1,10 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xmlns="http://logging.apache.org/log4j/changelog"; + xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.2.xsd"; + type="fixed"> + <issue id="LOG4J2-3680" link="https://issues.apache.org/jira/browse/LOG4J2-3680"/> + <description format="asciidoc"> + Allow deserialization of all arrays of allowed classes. + </description> +</entry> diff --git a/src/changelog/2.22.0/1906_harden_serialization_process.xml b/src/changelog/2.22.0/1906_harden_serialization_process.xml new file mode 100644 index 0000000000..0164b143ef --- /dev/null +++ b/src/changelog/2.22.0/1906_harden_serialization_process.xml @@ -0,0 +1,10 @@ +<?xml version="1.0" encoding="UTF-8"?> +<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; + xmlns="http://logging.apache.org/log4j/changelog"; + xsi:schemaLocation="http://logging.apache.org/log4j/changelog https://logging.apache.org/log4j/changelog-0.1.3.xsd"; + type="fixed"> + <issue id="1906" link="https://github.com/apache/logging-log4j2/issues/1884"/> + <description format="asciidoc"> + Harden deserialization process by requiring the usage of `FilteredObjectInputStream` on Java 8 and `ObjectInputFilter` on Java 9+ to deserialize custom classes. + </description> +</entry>
