Skip to content

Commit

Permalink
Improved: Improve UtilObject class (OFBIZ-12216)
Browse files Browse the repository at this point in the history
Removes "DiskFileItem, FileItemHeadersImpl are not serializable" case. It does
not appear in trunk.

Handling with exception Rather than returning null cleans UtilObject class.

Restrict unauthorized deserialisations to java.rmi instead of java.rmi.server
  • Loading branch information
JacquesLeRoux committed Mar 30, 2021
1 parent 643b9c7 commit 1bc8a20
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import java.io.IOException;
import java.io.InputStream;
import java.io.InvalidClassException;
import java.io.ObjectInputStream;
import java.io.ObjectStreamClass;
import java.util.Arrays;
Expand Down Expand Up @@ -64,20 +65,11 @@ public SafeObjectInputStream(InputStream in) throws IOException {
protected Class<?> resolveClass(ObjectStreamClass classDesc) throws IOException, ClassNotFoundException {
String className = classDesc.getName();
// DenyList exploits; eg: don't allow RMI here
if (className.contains("java.rmi.server")) {
Debug.logWarning("***Incompatible class***: "
+ classDesc.getName()
+ ". java.rmi.server classes are not allowed for security reason",
"SafeObjectInputStream");
return null;
if (className.contains("java.rmi")) {
throw new InvalidClassException(className, "Unauthorized deserialisation attempt");
}
if (!allowlistPattern.matcher(className).find()) {
// DiskFileItem, FileItemHeadersImpl are not serializable.
if (className.contains("org.apache.commons.fileupload")) {
return null;
}
Debug.logWarning("***Incompatible class***: "
+ classDesc.getName()
Debug.logWarning("***Incompatible class***: " + className
+ ". Please see OFBIZ-10837. Report to dev ML if you use OFBiz without changes. "
+ "Else follow https://s.apache.org/45war",
"SafeObjectInputStream");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,7 @@ public static Object getObject(byte[] bytes) {
Object obj = null;
try {
obj = getObjectException(bytes);
// DiskFileItem, FileItemHeadersImpl are not serializable. So SafeObjectInputStream::resolveClass return null
if (obj == null) {
return null;
}
} catch (ClassNotFoundException | IOException e) {
} catch (IOException | ClassCastException | ClassNotFoundException e) {
Debug.logError(e, MODULE);
}
return obj;
Expand All @@ -94,7 +90,7 @@ public static Object getObject(byte[] bytes) {
* @throws ClassNotFoundException when the class can not be deserialized.
* @throws IOException when a general Input/Output error happen.
*/
public static Object getObjectException(byte[] bytes) throws ClassNotFoundException, IOException {
public static Object getObjectException(byte[] bytes) throws ClassCastException, ClassNotFoundException, IOException {
try (ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
SafeObjectInputStream wois = new SafeObjectInputStream(bis)) {
return wois.readObject();
Expand Down

0 comments on commit 1bc8a20

Please sign in to comment.