Skip to content

Commit 2b50e38

Browse files
ppkarwaszppkarwasz
committed
[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.
1 parent 035e251 commit 2b50e38

File tree

18 files changed

+251
-188
lines changed

18 files changed

+251
-188
lines changed

‎log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/DefaultObjectInputFilter.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public static DefaultObjectInputFilter newInstance(final ObjectInputFilter filte
4444

4545
@Override
4646
public Status checkInput(final FilterInfo filterInfo) {
47-
Status status = null;
47+
Status status;
4848
if (delegate != null) {
4949
status = delegate.checkInput(filterInfo);
5050
if (status != Status.UNDECIDED) {
@@ -59,9 +59,10 @@ public Status checkInput(final FilterInfo filterInfo) {
5959
return status;
6060
}
6161
}
62-
if (filterInfo.serialClass() != null) {
63-
final String name = filterInfo.serialClass().getName();
64-
if (isAllowedByDefault(name) || isRequiredPackage(name)) {
62+
final Class<?> serialClass = filterInfo.serialClass();
63+
if (serialClass != null) {
64+
final String name = SerializationUtil.stripArray(serialClass);
65+
if (isAllowedByDefault(name)) {
6566
return Status.ALLOWED;
6667
}
6768
} else {

‎log4j-api-java9/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,8 @@
2424
public final class SerializationUtil {
2525
public static final List<String> REQUIRED_JAVA_CLASSES = List.of();
2626
public static final List<String> REQUIRED_JAVA_PACKAGES = List.of();
27+
28+
public static String stripArray(final Class<?> clazz) {
29+
return null;
30+
}
2731
}

‎log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/SerialUtil.java

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@
1919
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2020
import java.io.ByteArrayInputStream;
2121
import java.io.ByteArrayOutputStream;
22+
import java.io.IOException;
23+
import java.io.InputStream;
2224
import java.io.ObjectInputStream;
25+
import java.io.ObjectOutput;
2326
import java.io.ObjectOutputStream;
2427
import java.io.Serializable;
2528
import org.apache.logging.log4j.util.Constants;
@@ -38,10 +41,21 @@ private SerialUtil() {}
3841
* @return the serialized object
3942
*/
4043
public static byte[] serialize(final Serializable obj) {
44+
return serialize(new Serializable[] {obj});
45+
}
46+
47+
/**
48+
* Serializes the specified object and returns the result as a byte array.
49+
* @param objs an array of objects to serialize
50+
* @return the serialized object
51+
*/
52+
public static byte[] serialize(final Serializable... objs) {
4153
try {
4254
final ByteArrayOutputStream bas = new ByteArrayOutputStream(8192);
43-
final ObjectOutputStream oos = new ObjectOutputStream(bas);
44-
oos.writeObject(obj);
55+
final ObjectOutput oos = new ObjectOutputStream(bas);
56+
for (final Object obj : objs) {
57+
oos.writeObject(obj);
58+
}
4559
oos.flush();
4660
return bas.toByteArray();
4761
} catch (final Exception ex) {
@@ -58,16 +72,33 @@ public static byte[] serialize(final Serializable obj) {
5872
@SuppressFBWarnings("OBJECT_DESERIALIZATION")
5973
public static <T> T deserialize(final byte[] data) {
6074
try {
61-
final ByteArrayInputStream bas = new ByteArrayInputStream(data);
62-
final ObjectInputStream ois;
63-
if (Constants.JAVA_MAJOR_VERSION == 8) {
64-
ois = new FilteredObjectInputStream(bas);
65-
} else {
66-
ois = new ObjectInputStream(bas);
67-
}
75+
final ObjectInputStream ois = getObjectInputStream(data);
6876
return (T) ois.readObject();
6977
} catch (final Exception ex) {
7078
throw new IllegalStateException("Could not deserialize", ex);
7179
}
7280
}
81+
82+
/**
83+
* Creates an {@link ObjectInputStream} adapted to the current Java version.
84+
* @param data data to deserialize,
85+
* @return an object input stream.
86+
*/
87+
@SuppressFBWarnings("OBJECT_DESERIALIZATION")
88+
public static ObjectInputStream getObjectInputStream(final byte[] data) throws IOException {
89+
final ByteArrayInputStream bas = new ByteArrayInputStream(data);
90+
return getObjectInputStream(bas);
91+
}
92+
93+
/**
94+
* Creates an {@link ObjectInputStream} adapted to the current Java version.
95+
* @param stream stream of data to deserialize,
96+
* @return an object input stream.
97+
*/
98+
@SuppressFBWarnings("OBJECT_DESERIALIZATION")
99+
public static ObjectInputStream getObjectInputStream(final InputStream stream) throws IOException {
100+
return Constants.JAVA_MAJOR_VERSION == 8
101+
? new FilteredObjectInputStream(stream)
102+
: new ObjectInputStream(stream);
103+
}
73104
}

‎log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* limitations under the license.
1616
*/
1717
@Export
18-
@Version("2.21.1")
18+
@Version("2.23.0")
1919
package org.apache.logging.log4j.test.junit;
2020

2121
import org.osgi.annotation.bundle.Export;

‎log4j-api-test/src/test/java/org/apache/logging/log4j/message/ObjectMessageTest.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,13 @@ public boolean equals(final Object other) {
9292
return other instanceof NonSerializable; // a very lenient equals()
9393
}
9494
}
95-
return Stream.of("World", new NonSerializable(), new BigDecimal("123.456"), null);
95+
return Stream.of(
96+
"World",
97+
new NonSerializable(),
98+
new BigDecimal("123.456"),
99+
// LOG4J2-3680
100+
new RuntimeException(),
101+
null);
96102
}
97103

98104
@ParameterizedTest

‎log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import static org.assertj.core.api.Assertions.assertThat;
2020
import static org.junit.jupiter.api.Assertions.assertEquals;
2121

22+
import java.math.BigDecimal;
2223
import java.util.stream.Stream;
2324
import org.apache.logging.log4j.test.junit.Mutable;
2425
import org.apache.logging.log4j.test.junit.SerialUtil;
@@ -149,7 +150,20 @@ public void testSafeWithMutableParams() { // LOG4J2-763
149150
}
150151

151152
static Stream<Object> testSerializable() {
152-
return Stream.of("World", new Object(), null);
153+
@SuppressWarnings("EqualsHashCode")
154+
class NonSerializable {
155+
@Override
156+
public boolean equals(final Object other) {
157+
return other instanceof NonSerializable; // a very lenient equals()
158+
}
159+
}
160+
return Stream.of(
161+
"World",
162+
new NonSerializable(),
163+
new BigDecimal("123.456"),
164+
// LOG4J2-3680
165+
new RuntimeException(),
166+
null);
153167
}
154168

155169
@ParameterizedTest

‎log4j-api-test/src/test/java/org/apache/logging/log4j/util/SortedArrayStringMapTest.java

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
*/
1717
package org.apache.logging.log4j.util;
1818

19+
import static org.apache.logging.log4j.test.junit.SerialUtil.deserialize;
20+
import static org.apache.logging.log4j.test.junit.SerialUtil.serialize;
1921
import static org.junit.jupiter.api.Assertions.assertAll;
2022
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
2123
import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -27,14 +29,9 @@
2729
import static org.junit.jupiter.api.Assertions.fail;
2830

2931
import java.io.BufferedReader;
30-
import java.io.ByteArrayInputStream;
31-
import java.io.ByteArrayOutputStream;
3232
import java.io.File;
3333
import java.io.FileOutputStream;
34-
import java.io.IOException;
3534
import java.io.InputStreamReader;
36-
import java.io.ObjectInputStream;
37-
import java.io.ObjectOutputStream;
3835
import java.lang.reflect.Field;
3936
import java.net.URL;
4037
import java.net.URLDecoder;
@@ -191,20 +188,6 @@ private String createClassPath(final Class<?> cls) throws Exception {
191188
return location.isEmpty() ? "." : location;
192189
}
193190

194-
private byte[] serialize(final SortedArrayStringMap data) throws IOException {
195-
final ByteArrayOutputStream arr = new ByteArrayOutputStream();
196-
final ObjectOutputStream out = new ObjectOutputStream(arr);
197-
out.writeObject(data);
198-
return arr.toByteArray();
199-
}
200-
201-
private SortedArrayStringMap deserialize(final byte[] binary) throws IOException, ClassNotFoundException {
202-
final ByteArrayInputStream inArr = new ByteArrayInputStream(binary);
203-
try (final ObjectInputStream in = new FilteredObjectInputStream(inArr)) {
204-
return (SortedArrayStringMap) in.readObject();
205-
}
206-
}
207-
208191
@Test
209192
public void testPutAll() {
210193
final SortedArrayStringMap original = new SortedArrayStringMap();

‎log4j-api-test/src/test/java/org/apache/logging/log4j/util/internal/SerializationUtilTest.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to you under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.logging.log4j.util.internal;
18+
19+
import static org.assertj.core.api.Assertions.assertThat;
20+
21+
import java.util.stream.Stream;
22+
import org.junit.jupiter.params.ParameterizedTest;
23+
import org.junit.jupiter.params.provider.Arguments;
24+
import org.junit.jupiter.params.provider.MethodSource;
25+
26+
class SerializationUtilTest {
27+
28+
static Stream<Arguments> arrays() {
29+
return Stream.of(
30+
Arguments.of(boolean[].class, boolean.class),
31+
Arguments.of(char[].class, char.class),
32+
Arguments.of(byte[].class, byte.class),
33+
Arguments.of(short[].class, short.class),
34+
Arguments.of(int[].class, int.class),
35+
Arguments.of(long[].class, long.class),
36+
Arguments.of(float[].class, float.class),
37+
Arguments.of(double[].class, double.class),
38+
Arguments.of(String.class, String.class),
39+
Arguments.of(String[].class, String.class),
40+
Arguments.of(String[][].class, String.class));
41+
}
42+
43+
@ParameterizedTest
44+
@MethodSource("arrays")
45+
void stripArrayClass(final Class<?> arrayClass, final Class<?> componentClazz) {
46+
assertThat(SerializationUtil.stripArray(arrayClass)).isEqualTo(componentClazz.getName());
47+
}
48+
49+
@ParameterizedTest
50+
@MethodSource("arrays")
51+
void stripArrayString(final Class<?> arrayClass, final Class<?> componentClazz) {
52+
assertThat(SerializationUtil.stripArray(arrayClass.getName())).isEqualTo(componentClazz.getName());
53+
}
54+
}

‎log4j-api/src/main/java/org/apache/logging/log4j/util/FilteredObjectInputStream.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.io.ObjectStreamClass;
2727
import java.util.Collection;
2828
import java.util.Collections;
29+
import org.apache.logging.log4j.util.internal.SerializationUtil;
2930

3031
/**
3132
* Extends {@link ObjectInputStream} to only allow some built-in Log4j classes and caller-specified classes to be
@@ -63,7 +64,7 @@ public Collection<String> getAllowedClasses() {
6364

6465
@Override
6566
protected Class<?> resolveClass(final ObjectStreamClass desc) throws IOException, ClassNotFoundException {
66-
final String name = desc.getName();
67+
final String name = SerializationUtil.stripArray(desc.getName());
6768
if (!(isAllowedByDefault(name) || allowedExtraClasses.contains(name))) {
6869
throw new InvalidObjectException("Class is not allowed for deserialization: " + name);
6970
}

‎log4j-api/src/main/java/org/apache/logging/log4j/util/internal/SerializationUtil.java

Lines changed: 68 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,18 @@ public final class SerializationUtil {
7979
"java.math.BigInteger",
8080
// for Message delegate
8181
"java.rmi.MarshalledObject",
82-
"[B",
83-
// for MessagePatternAnalysis
84-
"[I");
82+
// all primitives
83+
"boolean",
84+
"byte",
85+
"char",
86+
"double",
87+
"float",
88+
"int",
89+
"long",
90+
"short");
8591

86-
public static final List<String> REQUIRED_JAVA_PACKAGES = Arrays.asList(
87-
"java.lang.", "java.time", "java.util.", "org.apache.logging.log4j.", "[Lorg.apache.logging.log4j.");
92+
public static final List<String> REQUIRED_JAVA_PACKAGES =
93+
Arrays.asList("java.lang.", "java.time.", "java.util.", "org.apache.logging.log4j.");
8894

8995
public static void writeWrappedObject(final Serializable obj, final ObjectOutputStream out) throws IOException {
9096
final ByteArrayOutputStream bout = new ByteArrayOutputStream();
@@ -133,5 +139,62 @@ public static void assertFiltered(final java.io.ObjectInputStream stream) {
133139
}
134140
}
135141

142+
/**
143+
* Gets the class name of an array component recursively.
144+
* <p>
145+
* If {@code clazz} is not an array class its name is returned.
146+
* </p>
147+
* @param clazz the binary name of a class.
148+
*/
149+
public static String stripArray(final Class<?> clazz) {
150+
Class<?> currentClazz = clazz;
151+
while (currentClazz.isArray()) {
152+
currentClazz = currentClazz.getComponentType();
153+
}
154+
return currentClazz.getName();
155+
}
156+
157+
/**
158+
* Gets the class name of an array component recursively.
159+
* <p>
160+
* If {@code name} is not the name of an array class it is returned unchanged.
161+
* </p>
162+
* @param name the name of a class.
163+
* @see Class#getName()
164+
*/
165+
public static String stripArray(final String name) {
166+
int offset;
167+
for (offset = 0; offset < name.length() && name.charAt(offset) == '['; offset++) {}
168+
if (offset == 0) {
169+
return name;
170+
}
171+
// Reference types
172+
if (name.charAt(offset) == 'L') {
173+
return name.substring(offset + 1, name.length() - 1);
174+
}
175+
// Primitive classes
176+
switch (name.substring(offset)) {
177+
case "Z":
178+
return "boolean";
179+
case "B":
180+
return "byte";
181+
case "C":
182+
return "char";
183+
case "D":
184+
return "double";
185+
case "F":
186+
return "float";
187+
case "I":
188+
return "int";
189+
case "J":
190+
return "long";
191+
case "S":
192+
return "short";
193+
default:
194+
// Should never happen
195+
return name;
196+
}
197+
}
198+
136199
private SerializationUtil() {}
137200
}

0 commit comments

Comments
 (0)