You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ofbiz.apache.org by jl...@apache.org on 2020/02/24 10:57:29 UTC
[ofbiz-framework] branch release18.12 updated: Fixed: Improve
ObjectInputStream class (CVE-2019-0189) Improved: no functional change
(OFBIZ-10837) (OFBIZ-11398)
This is an automated email from the ASF dual-hosted git repository.
jleroux pushed a commit to branch release18.12
in repository https://gitbox.apache.org/repos/asf/ofbiz-framework.git
The following commit(s) were added to refs/heads/release18.12 by this push:
new e78cc49 Fixed: Improve ObjectInputStream class (CVE-2019-0189) Improved: no functional change (OFBIZ-10837) (OFBIZ-11398)
e78cc49 is described below
commit e78cc49d713f40822608491230de8432aafdd875
Author: Jacques Le Roux <ja...@les7arts.com>
AuthorDate: Mon Feb 24 11:00:34 2020 +0100
Fixed: Improve ObjectInputStream class (CVE-2019-0189)
Improved: no functional change
(OFBIZ-10837) (OFBIZ-11398)
Steps to generate:
1. Navigate to - catalog/control/EditProdCatalog?prodCatalogId=TestCatalog
2. Click on - CREATE SEO CATEGORY/PRODUCTS
3. The broken page will be displayed
The issue is due to the use of a GString in
createMissingCategoryAndProductAltUrls().
This:
result.successMessageList = [
"Categories updated: ${categoriesUpdated}",
"Products updated: ${productsUpdated}"
As it's common to use such expressions I have added the necessary
org.codehaus.groovy.runtime.GStringImpl groovy.lang.GString
to the white list of classes in listOfSafeObjectsForInputStream in
SafeObjectInputStream.properties
I finally have also decided to use this property as default and commented for
committers to be aware that it should be also put in DEFAULT_WHITELIST_PATTERN
in SafeObjectInputStream class. Because if, for a reason,
listOfSafeObjectsForInputStream is empty OFBiz will still be protected
Thanks: Dikpal Kanungo for reporting
# Conflicts:
# SafeObjectInputStream.java
Handled by hand
---
.../base/config/SafeObjectInputStream.properties | 6 +-
.../ofbiz/base/util/SafeObjectInputStream.java | 93 +++++++++-------------
2 files changed, 43 insertions(+), 56 deletions(-)
diff --git a/framework/base/config/SafeObjectInputStream.properties b/framework/base/config/SafeObjectInputStream.properties
index bdc5b4a..548eab7 100644
--- a/framework/base/config/SafeObjectInputStream.properties
+++ b/framework/base/config/SafeObjectInputStream.properties
@@ -21,7 +21,9 @@
# If you encounter a related issue (object not in the whitelist),
# you must provide a complete list of objects to pass to ObjectInputStream
# through ListOfSafeObjectsForInputStream property
-# As an example, the a complete list of objects used by OFBiz OOTB is commented out by default here.
+# As an example, the a complete list of objects used by OFBiz OOTB is here.
# You will need to add your objects/classes to this list.
+# OFBiz committers: don't forget to add newobjects in SafeObjectInputStream class too (as default there).
-#listOfSafeObjectsForInputStream=byte\\\\[\\\\], foo, SerializationInjector, \\\\[Z,\\\\[B,\\\\[S,\\\\[I,\\\\[J,\\\\[F,\\\\[D,\\\\[C, java..*, sun.util.calendar..*, org.apache.ofbiz..*
+
+listOfSafeObjectsForInputStream=byte\\\\[\\\\], foo, SerializationInjector, \\\\[Z,\\\\[B,\\\\[S,\\\\[I,\\\\[J,\\\\[F,\\\\[D,\\\\[C, java..*, sun.util.calendar..*, org.apache.ofbiz..*, org.codehaus.groovy.runtime.GStringImpl, groovy.lang.GString
diff --git a/framework/base/src/main/java/org/apache/ofbiz/base/util/SafeObjectInputStream.java b/framework/base/src/main/java/org/apache/ofbiz/base/util/SafeObjectInputStream.java
index c48699a..2aebcde 100644
--- a/framework/base/src/main/java/org/apache/ofbiz/base/util/SafeObjectInputStream.java
+++ b/framework/base/src/main/java/org/apache/ofbiz/base/util/SafeObjectInputStream.java
@@ -1,4 +1,4 @@
-/*******************************************************************************
+/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
@@ -15,77 +15,62 @@
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
- *******************************************************************************/
+ */
package org.apache.ofbiz.base.util;
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.joining;
+import static org.apache.ofbiz.base.util.UtilProperties.getPropertyValue;
+
import java.io.IOException;
import java.io.InputStream;
+import java.io.ObjectInputStream;
import java.io.ObjectStreamClass;
-import java.lang.reflect.Proxy;
-import java.util.List;
+import java.util.Arrays;
import java.util.regex.Pattern;
/**
- * ObjectInputStream
+ * SafeObjectInputStream
*
+ * <p> An implementation of {@link java.io.ObjectInputStream} that ensure that
+ * only authorized class can be read from it.
*/
-public class SafeObjectInputStream extends java.io.ObjectInputStream {
-
- private ClassLoader classloader;
- private Pattern WHITELIST_PATTERN = null;
+public final class SafeObjectInputStream extends ObjectInputStream {
+ private static final String[] DEFAULT_WHITELIST_PATTERN = {
+ "byte\\[\\]", "foo", "SerializationInjector",
+ "\\[Z", "\\[B", "\\[S", "\\[I", "\\[J", "\\[F", "\\[D", "\\[C",
+ "java..*", "sun.util.calendar..*", "org.apache.ofbiz..*",
+ "org.codehaus.groovy.runtime.GStringImpl", "groovy.lang.GString"};
- public SafeObjectInputStream(InputStream in, ClassLoader loader) throws IOException {
- super(in);
- this.classloader = loader;
- }
+ /** The regular expression used to match serialized types. */
+ private final Pattern whitelistPattern;
- public SafeObjectInputStream(InputStream in, ClassLoader loader, List<String> whitelist) throws IOException {
+ /**
+ * Instantiates a safe object input stream.
+ *
+ * @param in the input stream to read
+ * @throws IOException when reading is not possible.
+ */
+ public SafeObjectInputStream(InputStream in) throws IOException {
super(in);
- this.classloader = loader;
- StringBuilder bld = new StringBuilder("(");
- for (int i = 0; i < whitelist.size(); i++) {
- bld.append(whitelist.get(i));
- if (i != whitelist.size() - 1) {
- bld.append("|");
- }
- }
- bld.append(")");
- WHITELIST_PATTERN = Pattern.compile(bld.toString());
+ String safeObjectsProp = getPropertyValue("SafeObjectInputStream", "ListOfSafeObjectsForInputStream", "");
+ String[] whitelist = safeObjectsProp.isEmpty() ? DEFAULT_WHITELIST_PATTERN : safeObjectsProp.split(",");
+ whitelistPattern = Arrays.stream(whitelist)
+ .map(String::trim)
+ .filter(str -> !str.isEmpty())
+ .collect(collectingAndThen(joining("|", "(", ")"), Pattern::compile));
}
-
- /**
- * @see java.io.ObjectInputStream#resolveClass(java.io.ObjectStreamClass)
- */
@Override
protected Class<?> resolveClass(ObjectStreamClass classDesc) throws IOException, ClassNotFoundException {
- if (!WHITELIST_PATTERN.matcher(classDesc.getName()).find()) {
- Debug.logWarning("***Incompatible class***: " + classDesc.getName() +
- ". Please see OFBIZ-10837. Report to dev ML if you use OFBiz without changes. "
- + "Else follow https://s.apache.org/45war"
- , "SafeObjectInputStream");
+ if (!whitelistPattern.matcher(classDesc.getName()).find()) {
+ Debug.logWarning("***Incompatible class***: "
+ + classDesc.getName()
+ + ". Please see OFBIZ-10837. Report to dev ML if you use OFBiz without changes. "
+ + "Else follow https://s.apache.org/45war",
+ "SafeObjectInputStream");
throw new ClassCastException("Incompatible class: " + classDesc.getName());
}
-
- return ObjectType.loadClass(classDesc.getName(), classloader);
- }
-
- /**
- * @see java.io.ObjectInputStream#resolveProxyClass(java.lang.String[])
- */
- @Override
- protected Class<?> resolveProxyClass(String[] interfaces) throws IOException, ClassNotFoundException {
- Class<?>[] cinterfaces = new Class<?>[interfaces.length];
- for (int i = 0; i < interfaces.length; i++) {
- cinterfaces[i] = classloader.loadClass(interfaces[i]);
- }
- //Proxy.getInvocationHandler(proxy)
-
- try {
- return Proxy.getProxyClass(classloader, cinterfaces);
- } catch (IllegalArgumentException e) {
- throw new ClassNotFoundException(null, e);
- }
-
+ return ObjectType.loadClass(classDesc.getName(), Thread.currentThread().getContextClassLoader());
}
}