From adf8691dcd56a9867d8e04b074ed586c06790723 Mon Sep 17 00:00:00 2001 From: Artem Smotrakov Date: Tue, 2 Apr 2019 12:16:03 +0200 Subject: [PATCH] HTTPCLIENT-1976: Unsafe deserialization in DefaultHttpCacheEntrySerializer --- .../cache/ByteArrayCacheEntrySerializer.java | 55 ++++++++++++++++++- .../TestByteArrayCacheEntrySerializer.java | 40 ++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ByteArrayCacheEntrySerializer.java b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ByteArrayCacheEntrySerializer.java index 7b7f45732..c15352cab 100644 --- a/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ByteArrayCacheEntrySerializer.java +++ b/httpclient5-cache/src/main/java/org/apache/hc/client5/http/impl/cache/ByteArrayCacheEntrySerializer.java @@ -29,8 +29,14 @@ package org.apache.hc.client5.http.impl.cache; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; +import java.io.ObjectStreamClass; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.regex.Pattern; import org.apache.hc.client5.http.cache.HttpCacheEntrySerializer; import org.apache.hc.client5.http.cache.HttpCacheStorageEntry; @@ -49,8 +55,24 @@ import org.apache.hc.core5.annotation.ThreadingBehavior; @Contract(threading = ThreadingBehavior.STATELESS) public final class ByteArrayCacheEntrySerializer implements HttpCacheEntrySerializer { + private static final List ALLOWED_CLASS_PATTERNS = Collections.unmodifiableList(Arrays.asList( + Pattern.compile("^(\\[L)?org\\.apache\\.hc\\.(.*)"), + Pattern.compile("^(\\[L)?java\\.util\\.(.*)"), + Pattern.compile("^(\\[L)?java\\.lang\\.(.*)$"), + Pattern.compile("^\\[B$"))); + public static final ByteArrayCacheEntrySerializer INSTANCE = new ByteArrayCacheEntrySerializer(); + private final List allowedClassPatterns; + + ByteArrayCacheEntrySerializer(final Pattern... allowedClassPatterns) { + this.allowedClassPatterns = Collections.unmodifiableList(Arrays.asList(allowedClassPatterns)); + } + + public ByteArrayCacheEntrySerializer() { + this.allowedClassPatterns = ALLOWED_CLASS_PATTERNS; + } + @Override public byte[] serialize(final HttpCacheStorageEntry cacheEntry) throws ResourceIOException { if (cacheEntry == null) { @@ -70,11 +92,42 @@ public final class ByteArrayCacheEntrySerializer implements HttpCacheEntrySerial if (serializedObject == null) { return null; } - try (final ObjectInputStream ois = new ObjectInputStream(new ByteArrayInputStream(serializedObject))) { + try (final ObjectInputStream ois = new RestrictedObjectInputStream( + new ByteArrayInputStream(serializedObject), allowedClassPatterns)) { return (HttpCacheStorageEntry) ois.readObject(); } catch (final IOException | ClassNotFoundException ex) { throw new ResourceIOException(ex.getMessage(), ex); } } + private static class RestrictedObjectInputStream extends ObjectInputStream { + + private final List allowedClassPatterns; + + private RestrictedObjectInputStream(final InputStream in, final List patterns) throws IOException { + super(in); + this.allowedClassPatterns = patterns; + } + + @Override + protected Class resolveClass(final ObjectStreamClass desc) throws IOException, ClassNotFoundException { + if (isProhibited(desc)) { + throw new ResourceIOException(String.format( + "Class %s is not allowed for deserialization", desc.getName())); + } + return super.resolveClass(desc); + } + + private boolean isProhibited(final ObjectStreamClass desc) { + if (allowedClassPatterns != null) { + for (final Pattern pattern : allowedClassPatterns) { + if (pattern.matcher(desc.getName()).matches()) { + return false; + } + } + } + return true; + } + } + } diff --git a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestByteArrayCacheEntrySerializer.java b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestByteArrayCacheEntrySerializer.java index b39149550..1f7d520c1 100644 --- a/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestByteArrayCacheEntrySerializer.java +++ b/httpclient5-cache/src/test/java/org/apache/hc/client5/http/impl/cache/TestByteArrayCacheEntrySerializer.java @@ -29,20 +29,27 @@ package org.apache.hc.client5.http.impl.cache; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.ObjectOutputStream; import java.nio.charset.Charset; import java.util.Date; import java.util.HashMap; import java.util.Map; +import java.util.regex.Pattern; import org.apache.commons.codec.binary.Base64; import org.apache.hc.client5.http.cache.HttpCacheEntry; import org.apache.hc.client5.http.cache.HttpCacheStorageEntry; +import org.apache.hc.client5.http.cache.ResourceIOException; import org.apache.hc.core5.http.Header; import org.apache.hc.core5.http.HttpStatus; import org.apache.hc.core5.http.message.BasicHeader; import org.junit.Before; import org.junit.Test; +import com.sun.rowset.JdbcRowSetImpl; + public class TestByteArrayCacheEntrySerializer { private static final Charset UTF8 = Charset.forName("UTF-8"); @@ -59,6 +66,39 @@ public class TestByteArrayCacheEntrySerializer { readWriteVerify(makeCacheEntryWithVariantMap("key")); } + @Test(expected = ResourceIOException.class) + public void throwExceptionIfUnsafeDeserialization() throws IOException { + impl.deserialize(serializeProhibitedObject()); + } + + @Test(expected = ResourceIOException.class) + public void allowClassesToBeDeserialized() throws IOException { + impl = new ByteArrayCacheEntrySerializer( + Pattern.compile("javax.sql.rowset.BaseRowSet"), + Pattern.compile("com.sun.rowset.JdbcRowSetImpl")); + impl.deserialize(serializeProhibitedObject()); + } + + @Test(expected = ResourceIOException.class) + public void allowClassesToBeDeserializedByRegex() throws IOException { + impl = new ByteArrayCacheEntrySerializer( + Pattern.compile(("^com\\.sun\\.rowset\\.(.*)")), + Pattern.compile("^javax\\.sql\\.rowset\\.BaseRowSet$")); + impl.deserialize(serializeProhibitedObject()); + } + + private byte[] serializeProhibitedObject() throws IOException { + final JdbcRowSetImpl jdbcRowSet = new JdbcRowSetImpl(); + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + final ObjectOutputStream oos = new ObjectOutputStream(baos); + try { + oos.writeObject(jdbcRowSet); + } finally { + oos.close(); + } + return baos.toByteArray(); + } + public void readWriteVerify(final HttpCacheStorageEntry writeEntry) throws Exception { // write the entry final byte[] bytes = impl.serialize(writeEntry);