diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectArrayMessageTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectArrayMessageTest.java index 8acd13b3d71..44ffc8a3377 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectArrayMessageTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectArrayMessageTest.java @@ -18,7 +18,12 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.IOException; +import java.io.InvalidObjectException; +import java.io.ObjectInputStream; +import java.nio.ByteBuffer; import org.apache.logging.log4j.test.junit.SerialUtil; import org.junit.jupiter.api.Test; @@ -51,4 +56,63 @@ void testSerializableRoundTripThroughFilteredStream() { final ObjectArrayMessage restored = SerialUtil.deserialize(SerialUtil.serialize(original)); assertArrayEquals(original.getParameters(), restored.getParameters()); } + + @Test + void testNonSerializableElementIsReplacedByItsStringForm() { + final ObjectArrayMessage original = new ObjectArrayMessage("A", new NonSerializable(), "C"); + final ObjectArrayMessage restored = SerialUtil.deserialize(SerialUtil.serialize(original)); + assertArrayEquals(new Object[] {"A", "non-serializable", "C"}, restored.getParameters()); + } + + @Test + void testDeserializationResizesBeyondInitialAllocation() { + // One element more than the bounded initial allocation of the deserialized array + final Object[] array = new Object[(1 << 8) + 1]; + for (int i = 0; i < array.length; i++) { + array[i] = String.format("%08x", i); + } + final ObjectArrayMessage restored = SerialUtil.deserialize(SerialUtil.serialize(new ObjectArrayMessage(array))); + assertArrayEquals(array, restored.getParameters()); + } + + @Test + void testDeserializationDoesNotPreallocateDeclaredLength() throws Exception { + // A forged stream declaring a huge length must fail on the missing data, not allocate for it. + final byte[] forged = + patchLength(SerialUtil.serialize(new ObjectArrayMessage("A", "B", "C")), 3, Integer.MAX_VALUE); + final ObjectInputStream ois = SerialUtil.getObjectInputStream(forged); + assertThrows(IOException.class, ois::readObject); + } + + @Test + void testDeserializationRejectsNegativeLength() throws Exception { + final byte[] forged = patchLength(SerialUtil.serialize(new ObjectArrayMessage("A", "B", "C")), 3, -1); + final ObjectInputStream ois = SerialUtil.getObjectInputStream(forged); + assertThrows(InvalidObjectException.class, ois::readObject); + } + + /** + * Returns a copy of the serialized form with the declared array length replaced. + *

+ * The length follows the default field data as a block-data record ({@code 0x77}, length {@code 0x04}), + * so it can be located by its original value. + *

+ */ + private static byte[] patchLength(final byte[] binary, final int length, final int newLength) { + final ByteBuffer buffer = ByteBuffer.wrap(binary.clone()); + for (int i = 0; i + 6 <= binary.length; i++) { + if (buffer.get(i) == 0x77 && buffer.get(i + 1) == 0x04 && buffer.getInt(i + 2) == length) { + buffer.putInt(i + 2, newLength); + return buffer.array(); + } + } + throw new AssertionError("Unable to locate the length field in the serialized form"); + } + + private static final class NonSerializable { + @Override + public String toString() { + return "non-serializable"; + } + } } 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 4bd5df91bef..c96e831d9c3 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 @@ -185,6 +185,18 @@ void testSerializable(final Object arg) { assertThat(actual.getFormattedMessage()).isEqualTo(expected.getFormattedMessage()); } + @Test + void testSerializableResizesBeyondInitialAllocation() { + // One argument more than the bounded initial allocation of the deserialized array + final Object[] args = new Object[(1 << 8) + 1]; + for (int i = 0; i < args.length; i++) { + args[i] = String.format("%08x", i); + } + final Message expected = new ParameterizedMessage("Hello!", args); + final Message actual = SerialUtil.deserialize(SerialUtil.serialize(expected)); + assertThat(actual.getParameters()).isEqualTo(args); + } + /** * In this test cases, constructed the following scenarios:
*

diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectArrayMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectArrayMessage.java index b30b51f647d..6153bc313f2 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectArrayMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ObjectArrayMessage.java @@ -120,7 +120,7 @@ public int hashCode() { private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException { SerializationUtil.assertFiltered(in); in.defaultReadObject(); - array = (Object[]) in.readObject(); + array = SerializationUtil.readWrappedObjects(in); } @Override @@ -130,6 +130,6 @@ public String toString() { private void writeObject(final ObjectOutputStream out) throws IOException { out.defaultWriteObject(); - out.writeObject(array); + SerializationUtil.writeWrappedObjects(array, out); } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java index a93b495e42e..b0e1edf56c8 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; -import java.io.Serializable; import java.util.Arrays; import java.util.Objects; import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; @@ -374,20 +373,12 @@ public String toString() { private void writeObject(final ObjectOutputStream out) throws IOException { out.defaultWriteObject(); - out.writeInt(args.length); - for (final Object arg : args) { - final Serializable serializableArg = arg instanceof Serializable ? (Serializable) arg : String.valueOf(arg); - SerializationUtil.writeWrappedObject(serializableArg, out); - } + SerializationUtil.writeWrappedObjects(args, out); } private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException { SerializationUtil.assertFiltered(in); in.defaultReadObject(); - final int argCount = in.readInt(); - args = new Object[argCount]; - for (int argIndex = 0; argIndex < args.length; argIndex++) { - args[argIndex] = SerializationUtil.readWrappedObject(in); - } + args = SerializationUtil.readWrappedObjects(in); } } 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 6eeca8189a9..b25d9e24ee4 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 @@ -19,6 +19,7 @@ import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InvalidObjectException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; @@ -90,6 +91,11 @@ public final class SerializationUtil { public static final List REQUIRED_JAVA_PACKAGES = Arrays.asList("java.lang.", "java.time.", "java.util.", "org.apache.logging.log4j."); + /** + * The maximum number of array elements pre-allocated by {@link #readWrappedObjects(ObjectInputStream)}. + */ + private static final int MAX_DESERIALIZATION_LENGTH = 1 << 8; + public static void writeWrappedObject(final Serializable obj, final ObjectOutputStream out) throws IOException { final ByteArrayOutputStream bout = new ByteArrayOutputStream(); try (final ObjectOutputStream oos = new ObjectOutputStream(bout)) { @@ -130,6 +136,46 @@ public static Object readWrappedObject(final ObjectInputStream in) throws IOExce } } + /** + * Writes the length of {@code array} followed by its elements, each wrapped with + * {@link #writeWrappedObject}. + *

+ * Elements that are not {@link Serializable} are replaced by their {@link String#valueOf(Object)} + * representation. + *

+ */ + public static void writeWrappedObjects(final Object[] array, final ObjectOutputStream out) throws IOException { + out.writeInt(array.length); + for (final Object item : array) { + writeWrappedObject(item instanceof Serializable ? (Serializable) item : String.valueOf(item), out); + } + } + + /** + * Reads an array written by {@link #writeWrappedObjects(Object[], ObjectOutputStream)}. + *

+ * The declared length is not trusted: at most {@value #MAX_DESERIALIZATION_LENGTH} elements are + * pre-allocated and the array grows as elements are actually read, so a forged length cannot force a + * large allocation. + *

+ */ + public static Object[] readWrappedObjects(final ObjectInputStream in) throws IOException, ClassNotFoundException { + final int length = in.readInt(); + if (length < 0) { + throw new InvalidObjectException("Illegal array length: " + length); + } + int allocated = Math.min(length, MAX_DESERIALIZATION_LENGTH); + Object[] array = new Object[allocated]; + for (int i = 0; i < length; i++) { + if (i == allocated) { + allocated = (int) Math.min(length, 2L * allocated); + array = Arrays.copyOf(array, allocated); + } + array[i] = readWrappedObject(in); + } + return array; + } + public static void assertFiltered(final java.io.ObjectInputStream stream) { if (!(stream instanceof FilteredObjectInputStream) && setObjectInputFilter == null) { throw new IllegalArgumentException( diff --git a/src/changelog/.2.x.x/4274_wrap_object_array_message_serialization.xml b/src/changelog/.2.x.x/4274_wrap_object_array_message_serialization.xml new file mode 100644 index 00000000000..031c20bc4cb --- /dev/null +++ b/src/changelog/.2.x.x/4274_wrap_object_array_message_serialization.xml @@ -0,0 +1,12 @@ + + + + + `ObjectArrayMessage` now serializes its elements with the same filtered mechanism as `ParameterizedMessage`, and both bound array allocation during deserialization. + +