From: Markus Koschany <apo@debian.org> Date: Fri, 7 Apr 2017 14:35:27 +0200 Subject: CVE-2017-5929 Bug-Debian: https://bugs.debian.org/857343 Origin: https://github.com/qos-ch/logback/commit/f46044b805bca91efe5fd6afe52257cd02f775f8 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 | 12 ++-- .../logback/classic/net/SimpleSocketServer.java | 1 - .../ch/qos/logback/classic/net/SocketAppender.java | 2 - .../ch/qos/logback/classic/net/SocketNode.java | 15 +++-- .../server/HardenedLoggingEventInputStream.java | 56 +++++++++++++++++ .../net/server/RemoteAppenderStreamClient.java | 10 +-- .../core/net/HardenedObjectInputStream.java | 71 ++++++++++++++++++++++ 8 files changed, 159 insertions(+), 23 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 create mode 100644 logback-core/src/main/java/ch/qos/logback/core/net/HardenedObjectInputStream.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 32c6654..7db96a3 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,16 +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 +59,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 +79,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/SimpleSocketServer.java b/logback-classic/src/main/java/ch/qos/logback/classic/net/SimpleSocketServer.java index e450fff..a0fd7d8 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/net/SimpleSocketServer.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/net/SimpleSocketServer.java @@ -14,7 +14,6 @@ package ch.qos.logback.classic.net; import java.io.IOException; -import java.lang.reflect.Constructor; import java.net.ServerSocket; import java.net.Socket; import java.util.ArrayList; 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 1bc74a4..9f2ebef 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 98023e8..dcd967c 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,8 +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; @@ -81,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()); @@ -111,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/RemoteAppenderStreamClient.java b/logback-classic/src/main/java/ch/qos/logback/classic/net/server/RemoteAppenderStreamClient.java index a4b34a9..3901dad 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; /** @@ -86,7 +86,7 @@ class RemoteAppenderStreamClient implements RemoteAppenderClient { */ public void run() { logger.info(this + ": connected"); - ObjectInputStream ois = null; + HardenedObjectInputStream ois = null; try { ois = createObjectInputStream(); while (true) { @@ -124,11 +124,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 new file mode 100644 index 0000000..d1b7301 --- /dev/null +++ b/logback-core/src/main/java/ch/qos/logback/core/net/HardenedObjectInputStream.java @@ -0,0 +1,71 @@ +package ch.qos.logback.core.net; + +import java.io.IOException; +import java.io.InputStream; +import java.io.InvalidClassException; +import java.io.ObjectInputStream; +import java.io.ObjectStreamClass; +import java.util.ArrayList; +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 { + + 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 = 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)) { + throw new InvalidClassException("Unauthorized deserialization attempt", anObjectStreamClass.getName()); + } + + return super.resolveClass(anObjectStreamClass); + } + + private boolean isWhitelisted(String incomingClassName) { + for (int i = 0; i < JAVA_PACKAGES.length; i++) { + if (incomingClassName.startsWith(JAVA_PACKAGES[i])) + return true; + } + for (String whiteListed : whitelistedClassNames) { + if (incomingClassName.equals(whiteListed)) + return true; + } + return false; + } + + protected void addToWhitelist(List<String> additionalAuthorizedClasses) { + whitelistedClassNames.addAll(additionalAuthorizedClasses); + } +}