From: Markus Koschany <apo@debian.org>
Date: Tue, 4 Apr 2017 14:22:43 +0200
Subject: CVE-2017-5929-part2
This is part2 to fix CVE-2017-5929
Origin: https://github.com/qos-ch/logback/commit/979b042cb1f0b4c1e5869ccc8912e68c39f769f9
Origin: https://github.com/qos-ch/logback/commit/7fbea6127fa98fc48368ca5e8540eefe0e60cec5
Origin: https://github.com/qos-ch/logback/commit/3b4f605454534b304770eeee3cb343521fcd6968
---
.../access/net/HardenedAccessEventInputStream.java | 15 ++++++
.../java/ch/qos/logback/access/net/SocketNode.java | 11 ++---
.../ch/qos/logback/classic/net/SocketAppender.java | 2 -
.../ch/qos/logback/classic/net/SocketNode.java | 14 +++---
.../server/HardenedLoggingEventInputStream.java | 56 ++++++++++++++++++++++
.../server/LogbackClassicSerializationHelper.java | 28 -----------
.../net/server/RemoteAppenderStreamClient.java | 10 ++--
.../core/net/HardenedObjectInputStream.java | 47 +++++++++++++-----
8 files changed, 123 insertions(+), 60 deletions(-)
create mode 100644 logback-access/src/main/java/ch/qos/logback/access/net/HardenedAccessEventInputStream.java
create mode 100644 logback-classic/src/main/java/ch/qos/logback/classic/net/server/HardenedLoggingEventInputStream.java
delete mode 100644 logback-classic/src/main/java/ch/qos/logback/classic/net/server/LogbackClassicSerializationHelper.java
diff --git a/logback-access/src/main/java/ch/qos/logback/access/net/HardenedAccessEventInputStream.java b/logback-access/src/main/java/ch/qos/logback/access/net/HardenedAccessEventInputStream.java
new file mode 100644
index 0000000..c0ba6b0
--- /dev/null
+++ b/logback-access/src/main/java/ch/qos/logback/access/net/HardenedAccessEventInputStream.java
@@ -0,0 +1,15 @@
+package ch.qos.logback.access.net;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+import ch.qos.logback.access.spi.AccessEvent;
+import ch.qos.logback.core.net.HardenedObjectInputStream;
+
+public class HardenedAccessEventInputStream extends HardenedObjectInputStream {
+
+ public HardenedAccessEventInputStream(InputStream in) throws IOException {
+ super(in, new String[] {AccessEvent.class.getName(), String[].class.getName()});
+ }
+
+}
diff --git a/logback-access/src/main/java/ch/qos/logback/access/net/SocketNode.java b/logback-access/src/main/java/ch/qos/logback/access/net/SocketNode.java
index e164774..aeb7b14 100644
--- a/logback-access/src/main/java/ch/qos/logback/access/net/SocketNode.java
+++ b/logback-access/src/main/java/ch/qos/logback/access/net/SocketNode.java
@@ -15,7 +15,6 @@ package ch.qos.logback.access.net;
import java.io.BufferedInputStream;
import java.io.IOException;
-import java.io.ObjectInputStream;
import java.net.Socket;
import ch.qos.logback.access.spi.AccessContext;
@@ -42,15 +41,15 @@ public class SocketNode implements Runnable {
Socket socket;
AccessContext context;
- ObjectInputStream ois;
+ HardenedAccessEventInputStream hardenedOIS;
public SocketNode(Socket socket, AccessContext context) {
this.socket = socket;
this.context = context;
try {
- ois = new ObjectInputStream(new BufferedInputStream(socket.getInputStream()));
+ hardenedOIS = new HardenedAccessEventInputStream(new BufferedInputStream(socket.getInputStream()));
} catch (Exception e) {
- System.out.println("Could not open ObjectInputStream to " + socket + e);
+ System.out.println("Could not open HardenedObjectInputStream to " + socket + e);
}
}
@@ -61,7 +60,7 @@ public class SocketNode implements Runnable {
try {
while (true) {
// read an event from the wire
- event = (IAccessEvent) ois.readObject();
+ event = (IAccessEvent) hardenedOIS.readObject();
// check that the event should be logged
if (context.getFilterChainDecision(event) == FilterReply.DENY) {
break;
@@ -81,7 +80,7 @@ public class SocketNode implements Runnable {
}
try {
- ois.close();
+ hardenedOIS.close();
} catch (Exception e) {
System.out.println("Could not close connection." + e);
}
diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/net/SocketAppender.java b/logback-classic/src/main/java/ch/qos/logback/classic/net/SocketAppender.java
index 82518c7..0590cae 100644
--- a/logback-classic/src/main/java/ch/qos/logback/classic/net/SocketAppender.java
+++ b/logback-classic/src/main/java/ch/qos/logback/classic/net/SocketAppender.java
@@ -14,8 +14,6 @@
// Contributors: Dan MacDonald <dan@redknee.com>
package ch.qos.logback.classic.net;
-import java.net.InetAddress;
-
import ch.qos.logback.classic.spi.ILoggingEvent;
import ch.qos.logback.core.net.AbstractSocketAppender;
import ch.qos.logback.core.spi.PreSerializationTransformer;
diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/net/SocketNode.java b/logback-classic/src/main/java/ch/qos/logback/classic/net/SocketNode.java
index 4c01cbe..8faf6a6 100644
--- a/logback-classic/src/main/java/ch/qos/logback/classic/net/SocketNode.java
+++ b/logback-classic/src/main/java/ch/qos/logback/classic/net/SocketNode.java
@@ -15,13 +15,13 @@ package ch.qos.logback.classic.net;
import java.io.BufferedInputStream;
import java.io.IOException;
-import java.io.ObjectInputStream;
import java.net.Socket;
import java.net.SocketAddress;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.LoggerContext;
+import ch.qos.logback.classic.net.server.HardenedLoggingEventInputStream;
import ch.qos.logback.classic.spi.ILoggingEvent;
// Contributors: Moses Hohman <mmhohman@rainbow.uchicago.edu>
@@ -44,7 +44,7 @@ public class SocketNode implements Runnable {
Socket socket;
LoggerContext context;
- ObjectInputStream ois;
+ HardenedLoggingEventInputStream hardenedLoggingEventInputStream;
SocketAddress remoteSocketAddress;
Logger logger;
@@ -68,7 +68,7 @@ public class SocketNode implements Runnable {
public void run() {
try {
- ois = new ObjectInputStream(new BufferedInputStream(socket.getInputStream()));
+ hardenedLoggingEventInputStream = new HardenedLoggingEventInputStream(new BufferedInputStream(socket.getInputStream()));
} catch (Exception e) {
logger.error("Could not open ObjectInputStream to " + socket, e);
closed = true;
@@ -80,7 +80,7 @@ public class SocketNode implements Runnable {
try {
while (!closed) {
// read an event from the wire
- event = (ILoggingEvent) ois.readObject();
+ event = (ILoggingEvent) hardenedLoggingEventInputStream.readObject();
// get a logger from the hierarchy. The name of the logger is taken to
// be the name contained in the event.
remoteLogger = context.getLogger(event.getLoggerName());
@@ -110,13 +110,13 @@ public class SocketNode implements Runnable {
return;
}
closed = true;
- if (ois != null) {
+ if (hardenedLoggingEventInputStream != null) {
try {
- ois.close();
+ hardenedLoggingEventInputStream.close();
} catch (IOException e) {
logger.warn("Could not close connection.", e);
} finally {
- ois = null;
+ hardenedLoggingEventInputStream = null;
}
}
}
diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/net/server/HardenedLoggingEventInputStream.java b/logback-classic/src/main/java/ch/qos/logback/classic/net/server/HardenedLoggingEventInputStream.java
new file mode 100644
index 0000000..522a30f
--- /dev/null
+++ b/logback-classic/src/main/java/ch/qos/logback/classic/net/server/HardenedLoggingEventInputStream.java
@@ -0,0 +1,56 @@
+package ch.qos.logback.classic.net.server;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.slf4j.helpers.BasicMarker;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import ch.qos.logback.classic.spi.ClassPackagingData;
+import ch.qos.logback.classic.spi.IThrowableProxy;
+import ch.qos.logback.classic.spi.LoggerContextVO;
+import ch.qos.logback.classic.spi.LoggerRemoteView;
+import ch.qos.logback.classic.spi.LoggingEventVO;
+import ch.qos.logback.classic.spi.StackTraceElementProxy;
+import ch.qos.logback.classic.spi.ThrowableProxy;
+import ch.qos.logback.classic.spi.ThrowableProxyVO;
+import ch.qos.logback.core.net.HardenedObjectInputStream;
+
+public class HardenedLoggingEventInputStream extends HardenedObjectInputStream {
+
+ static final String ARRAY_PREFIX = "[L";
+
+ static public List<String> getWhilelist() {
+ List<String> whitelist = new ArrayList<String>();
+ whitelist.add(LoggingEventVO.class.getName());
+ whitelist.add(LoggerContextVO.class.getName());
+ whitelist.add(LoggerRemoteView.class.getName());
+ whitelist.add(ThrowableProxyVO.class.getName());
+ whitelist.add(BasicMarker.class.getName());
+ whitelist.add(Level.class.getName());
+ whitelist.add(Logger.class.getName());
+ whitelist.add(StackTraceElement.class.getName());
+ whitelist.add(StackTraceElement[].class.getName());
+ whitelist.add(ThrowableProxy.class.getName());
+ whitelist.add(ThrowableProxy[].class.getName());
+ whitelist.add(IThrowableProxy.class.getName());
+ whitelist.add(IThrowableProxy[].class.getName());
+ whitelist.add(StackTraceElementProxy.class.getName());
+ whitelist.add(StackTraceElementProxy[].class.getName());
+ whitelist.add(ClassPackagingData.class.getName());
+
+ return whitelist;
+ }
+
+ public HardenedLoggingEventInputStream(InputStream is) throws IOException {
+ super(is, getWhilelist());
+ }
+
+ public HardenedLoggingEventInputStream(InputStream is, List<String> additionalAuthorizedClasses) throws IOException {
+ this(is);
+ super.addToWhitelist(additionalAuthorizedClasses);
+ }
+}
diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/net/server/LogbackClassicSerializationHelper.java b/logback-classic/src/main/java/ch/qos/logback/classic/net/server/LogbackClassicSerializationHelper.java
deleted file mode 100644
index 00a974f..0000000
--- a/logback-classic/src/main/java/ch/qos/logback/classic/net/server/LogbackClassicSerializationHelper.java
+++ /dev/null
@@ -1,28 +0,0 @@
-package ch.qos.logback.classic.net.server;
-
-import java.util.ArrayList;
-import java.util.List;
-
-import org.slf4j.helpers.BasicMarker;
-
-import ch.qos.logback.classic.Logger;
-import ch.qos.logback.classic.spi.LoggerContextVO;
-import ch.qos.logback.classic.spi.LoggingEventVO;
-import ch.qos.logback.classic.spi.ThrowableProxyVO;
-
-public class LogbackClassicSerializationHelper {
-
-
-
- static public List<String> getWhilelist() {
- List<String> whitelist = new ArrayList<String>();
- whitelist.add(LoggingEventVO.class.getName());
- whitelist.add(LoggerContextVO.class.getName());
- whitelist.add(ThrowableProxyVO.class.getName());
- whitelist.add(StackTraceElement.class.getName());
- whitelist.add(BasicMarker.class.getName());
- whitelist.add(BasicMarker.class.getName());
- whitelist.add(Logger.class.getName());
- return whitelist;
- }
-}
diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/net/server/RemoteAppenderStreamClient.java b/logback-classic/src/main/java/ch/qos/logback/classic/net/server/RemoteAppenderStreamClient.java
index 5be7e24..71e1b0b 100644
--- a/logback-classic/src/main/java/ch/qos/logback/classic/net/server/RemoteAppenderStreamClient.java
+++ b/logback-classic/src/main/java/ch/qos/logback/classic/net/server/RemoteAppenderStreamClient.java
@@ -16,12 +16,12 @@ package ch.qos.logback.classic.net.server;
import java.io.EOFException;
import java.io.IOException;
import java.io.InputStream;
-import java.io.ObjectInputStream;
import java.net.Socket;
import ch.qos.logback.classic.Logger;
import ch.qos.logback.classic.LoggerContext;
import ch.qos.logback.classic.spi.ILoggingEvent;
+import ch.qos.logback.core.net.HardenedObjectInputStream;
import ch.qos.logback.core.util.CloseUtil;
/**
@@ -87,7 +87,7 @@ class RemoteAppenderStreamClient implements RemoteAppenderClient {
*/
public void run() {
logger.info(this + ": connected");
- ObjectInputStream ois = null;
+ HardenedObjectInputStream ois = null;
try {
ois = createObjectInputStream();
while (true) {
@@ -120,11 +120,11 @@ class RemoteAppenderStreamClient implements RemoteAppenderClient {
}
}
- private ObjectInputStream createObjectInputStream() throws IOException {
+ private HardenedObjectInputStream createObjectInputStream() throws IOException {
if (inputStream != null) {
- return new ObjectInputStream(inputStream);
+ return new HardenedLoggingEventInputStream(inputStream);
}
- return new ObjectInputStream(socket.getInputStream());
+ return new HardenedLoggingEventInputStream(socket.getInputStream());
}
/**
diff --git a/logback-core/src/main/java/ch/qos/logback/core/net/HardenedObjectInputStream.java b/logback-core/src/main/java/ch/qos/logback/core/net/HardenedObjectInputStream.java
index 439e2bd..d1b7301 100644
--- a/logback-core/src/main/java/ch/qos/logback/core/net/HardenedObjectInputStream.java
+++ b/logback-core/src/main/java/ch/qos/logback/core/net/HardenedObjectInputStream.java
@@ -6,43 +6,66 @@ import java.io.InvalidClassException;
import java.io.ObjectInputStream;
import java.io.ObjectStreamClass;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.List;
/**
+ * HardenedObjectInputStream restricts the set of classes that can be deserialized to a set of
+ * explicitly whitelisted classes. This prevents certain type of attacks from being successful.
+ *
+ * <p>It is assumed that classes in the "java.lang" and "java.util" packages are
+ * always authorized.</p>
*
* @author Ceki Gülcü
* @since 1.2.0
*/
public class HardenedObjectInputStream extends ObjectInputStream {
- List<String> whitelistedClassNames;
- String[] javaPackages = new String[] {"java.lang", "java.util"};
-
- public HardenedObjectInputStream(InputStream in, List<String> whilelist) throws IOException {
+ final List<String> whitelistedClassNames;
+ final static String[] JAVA_PACKAGES = new String[] { "java.lang", "java.util" };
+
+ public HardenedObjectInputStream(InputStream in, String[] whilelist) throws IOException {
super(in);
- this.whitelistedClassNames = Collections.synchronizedList(new ArrayList<String>(whilelist));
+
+ this.whitelistedClassNames = new ArrayList<String>();
+ if (whilelist != null) {
+ for (int i = 0; i < whilelist.length; i++) {
+ this.whitelistedClassNames.add(whilelist[i]);
+ }
+ }
+ }
+
+ public HardenedObjectInputStream(InputStream in, List<String> whitelist) throws IOException {
+ super(in);
+
+ this.whitelistedClassNames = new ArrayList<String>();
+ this.whitelistedClassNames.addAll(whitelist);
}
@Override
protected Class<?> resolveClass(ObjectStreamClass anObjectStreamClass) throws IOException, ClassNotFoundException {
+
String incomingClassName = anObjectStreamClass.getName();
- if(!isWhitelisted(incomingClassName)) {
+
+ if (!isWhitelisted(incomingClassName)) {
throw new InvalidClassException("Unauthorized deserialization attempt", anObjectStreamClass.getName());
}
-
+
return super.resolveClass(anObjectStreamClass);
}
private boolean isWhitelisted(String incomingClassName) {
- for(int i = 0; i < javaPackages.length; i++) {
- if(incomingClassName.startsWith(javaPackages[i]))
+ for (int i = 0; i < JAVA_PACKAGES.length; i++) {
+ if (incomingClassName.startsWith(JAVA_PACKAGES[i]))
return true;
}
- for(String className: whitelistedClassNames) {
- if(incomingClassName.equals(className))
+ for (String whiteListed : whitelistedClassNames) {
+ if (incomingClassName.equals(whiteListed))
return true;
}
return false;
}
+
+ protected void addToWhitelist(List<String> additionalAuthorizedClasses) {
+ whitelistedClassNames.addAll(additionalAuthorizedClasses);
+ }
}
