j3o hardening

Author: riccardobl

.github/workflows/j3o-scan.yml

@@ -0,0 +1,40 @@
+name: J3O Scan
+
+on:
+  pull_request:
+    paths:
+      - '**/*.j3o'
+      - 'jme3-desktop/src/main/resources/com/jme3/system/j3o-baseline.txt'
+      - 'jme3-desktop/src/main/java/com/jme3/system/J3OScanner.java'
+      - 'jme3-desktop/build.gradle'
+      - '.github/workflows/j3o-scan.yml'
+  push:
+    branches:
+      - master
+      - main
+    paths:
+      - '**/*.j3o'
+      - 'jme3-desktop/src/main/resources/com/jme3/system/j3o-baseline.txt'
+      - 'jme3-desktop/src/main/java/com/jme3/system/J3OScanner.java'
+      - 'jme3-desktop/build.gradle'
+      - '.github/workflows/j3o-scan.yml'
+
+permissions:
+  contents: read
+
+jobs:
+  scan-j3o:
+    name: Validate J3O assets
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+
+      - name: Set up Java
+        uses: actions/setup-java@v4
+        with:
+          distribution: temurin
+          java-version: '17'
+
+      - name: Scan J3O assets
+        run: ./gradlew :jme3-desktop:scanJ3O --console=plain

jme3-core/src/main/java/com/jme3/export/SavableClassFilter.java

@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2009-2026 jMonkeyEngine
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ *   notice, this list of conditions and the following disclaimer in the
+ *   documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of 'jMonkeyEngine' nor the names of its contributors
+ *   may be used to endorse or promote products derived from this software
+ *   without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
+ * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package com.jme3.export;
+
+/**
+ * Policy hook used before an importer instantiates classes named by a
+ * serialized asset.
+ */
+public interface SavableClassFilter {
+
+    /** Compatibility policy that allows every class name. */
+    SavableClassFilter ACCEPT_ALL = new SavableClassFilter() {
+        @Override
+        public boolean isAllowed(String className) {
+            return true;
+        }
+    };
+
+    /**
+     * @param className fully-qualified binary class name after legacy remapping
+     * @return true if the class may be instantiated
+     */
+    boolean isAllowed(String className);
+}

jme3-core/src/main/java/com/jme3/export/SavableClassUtil.java

@@ -86,7 +86,7 @@ private static void addRemapping(String oldClass, Class<? extends Savable> newCl
     private SavableClassUtil() {
     }
 
-    private static String remapClass(String className) throws ClassNotFoundException {
+    public static String remapClass(String className) {
         String result = CLASS_REMAPPINGS.get(className);
         if (result == null) {
             return className;
@@ -182,7 +182,32 @@ public static int getSavedSavableVersion(Object savable,
     public static Savable fromName(String className)
             throws ClassNotFoundException, IllegalAccessException,
             InstantiationException, InvocationTargetException {
+        return fromName(className, SavableClassFilter.ACCEPT_ALL);
+    }
+
+    /**
+     * Creates a new Savable after checking the remapped class name against the
+     * supplied filter.
+     *
+     * @param className the class name to create.
+     * @param classFilter policy controlling whether the class may be loaded.
+     * @return the Savable instance of the class.
+     * @throws InstantiationException thrown if the class does not have an empty constructor.
+     * @throws IllegalAccessException thrown if the class is not accessible.
+     * @throws InvocationTargetException if the underlying constructor throws an exception
+     * @throws ClassNotFoundException thrown if the class name is not in the classpath.
+     * @throws SecurityException thrown if the filter rejects the class name.
+     */
+    public static Savable fromName(String className, SavableClassFilter classFilter)
+            throws ClassNotFoundException, IllegalAccessException,
+            InstantiationException, InvocationTargetException {
         className = remapClass(className);
+        if (classFilter == null) {
+            throw new NullPointerException("classFilter");
+        }
+        if (!classFilter.isAllowed(className)) {
+            throw new SecurityException("Savable class rejected by filter: " + className);
+        }
 
         Constructor noArgConstructor = findNoArgConstructor(className);
         noArgConstructor.setAccessible(true);
@@ -241,6 +266,9 @@ public static Savable fromName(String className, List<ClassLoader> loaders) thro
     private static Constructor findNoArgConstructor(String className)
             throws ClassNotFoundException, InstantiationException {
         Class clazz = Class.forName(className);
+        if (!SavableClassUtil.isImplementingSavable(clazz)) {
+            throw new InstantiationException("Class " + className + " does not implement Savable.");
+        }
         Constructor result;
         try {
             result = clazz.getDeclaredConstructor();

jme3-core/src/plugins/java/com/jme3/export/binary/BinaryImporter.java

@@ -52,6 +52,7 @@ public final class BinaryImporter implements JmeImporter {
             .getName());
 
     private AssetManager assetManager;
+    private SavableClassFilter classFilter = SavableClassFilter.ACCEPT_ALL;
 
     //Key - alias, object - bco
     private final HashMap<String, BinaryClassObject> classes
@@ -99,6 +100,27 @@ public AssetManager getAssetManager(){
         return assetManager;
     }
 
+    /**
+     * Sets the policy used before this importer instantiates classes named by
+     * a J3O file. The default accepts all classes for backward compatibility.
+     * Use a restrictive filter when loading assets from untrusted sources.
+     *
+     * @param classFilter the filter to apply
+     */
+    public void setClassFilter(SavableClassFilter classFilter) {
+        if (classFilter == null) {
+            throw new NullPointerException("classFilter");
+        }
+        this.classFilter = classFilter;
+    }
+
+    /**
+     * @return the current class filter
+     */
+    public SavableClassFilter getClassFilter() {
+        return classFilter;
+    }
+
     @Override
     public Object load(AssetInfo info){
 //        if (!(info.getKey() instanceof ModelKey))
@@ -145,6 +167,9 @@ public Savable load(InputStream is, ReadListener listener, ByteArrayOutputStream
             formatVersion = ByteUtils.readInt(bis);
             numClasses = ByteUtils.readInt(bis);
 
+            if (formatVersion < 0) {
+                throw new IOException("Invalid J3O format version: " + formatVersion);
+            }
             // check if this binary is from the future
             if (formatVersion > FormatVersion.VERSION){
                 throw new IOException("The binary file is of newer version than expected! " + 
@@ -161,6 +186,9 @@ public Savable load(InputStream is, ReadListener listener, ByteArrayOutputStream
         }
 
         int bytes = 4;
+        if (numClasses <= 0) {
+            throw new IOException("Invalid J3O class count: " + numClasses);
+        }
         aliasWidth = ((int)FastMath.log(numClasses, 256) + 1);
 
         classes.clear();
@@ -170,7 +198,7 @@ public Savable load(InputStream is, ReadListener listener, ByteArrayOutputStream
             // jME3 NEW: Read class version number
             int[] classHierarchyVersions;
             if (formatVersion >= 1){
-                int classHierarchySize = bis.read();
+                int classHierarchySize = readUnsignedByte(bis, "class hierarchy size");
                 classHierarchyVersions = new int[classHierarchySize];
                 for (int j = 0; j < classHierarchySize; j++){
                     classHierarchyVersions[j] = ByteUtils.readInt(bis);
@@ -181,23 +209,29 @@ public Savable load(InputStream is, ReadListener listener, ByteArrayOutputStream
 
             // read classname and classname size
             int classLength = ByteUtils.readInt(bis);
+            checkLength(classLength);
             String className = readString(bis, classLength);
+            if (!classFilter.isAllowed(SavableClassUtil.remapClass(className))) {
+                throw new IOException("J3O class rejected by filter: " + className);
+            }
 
             BinaryClassObject bco = new BinaryClassObject();
             bco.alias = alias.getBytes();
             bco.className = className;
             bco.classHierarchyVersions = classHierarchyVersions;
 
             int fields = ByteUtils.readInt(bis);
+            checkLength(fields);
             bytes += (8 + aliasWidth + classLength);
 
             bco.nameFields = new HashMap<String, BinaryClassField>(fields);
             bco.aliasFields = new HashMap<Byte, BinaryClassField>(fields);
             for (int x = 0; x < fields; x++) {
-                byte fieldAlias = (byte)bis.read();
-                byte fieldType = (byte)bis.read();
+                byte fieldAlias = (byte) readUnsignedByte(bis, "field alias");
+                byte fieldType = (byte) readUnsignedByte(bis, "field type");
 
                 int fieldNameLength = ByteUtils.readInt(bis);
+                checkLength(fieldNameLength);
                 String fieldName = readString(bis, fieldNameLength);
                 BinaryClassField bcf = new BinaryClassField(fieldName, fieldAlias, fieldType);
                 bco.nameFields.put(fieldName, bcf);
@@ -209,13 +243,17 @@ public Savable load(InputStream is, ReadListener listener, ByteArrayOutputStream
         if (listener != null) listener.readBytes(bytes);
 
         int numLocs = ByteUtils.readInt(bis);
+        checkLength(numLocs);
         bytes = 4;
 
         capsuleTable.clear();
         locationTable.clear();
         for(int i = 0; i < numLocs; i++) {
             int id = ByteUtils.readInt(bis);
             int loc = ByteUtils.readInt(bis);
+            if (loc < 0) {
+                throw new IOException("Invalid negative J3O object location: " + loc);
+            }
             locationTable.put(id, loc);
             bytes += 8;
         }
@@ -290,15 +328,20 @@ public InputCapsule getCapsule(Savable id) {
     }
 
     protected String readString(InputStream f, int length) throws IOException {
+        checkLength(length);
         byte[] data = new byte[length];
         for(int j = 0; j < length; j++) {
-            data[j] = (byte)f.read();
+            data[j] = (byte) readUnsignedByte(f, "string");
         }
 
         return new String(data);
     }
 
     protected String readString(int length, int offset) throws IOException {
+        checkLength(length);
+        if (offset < 0 || offset + length < offset || offset + length > dataArray.length) {
+            throw new IOException("String outside J3O payload: offset=" + offset + ", length=" + length);
+        }
         byte[] data = new byte[length];
         for(int j = 0; j < length; j++) {
             data[j] = dataArray[j+offset];
@@ -307,14 +350,35 @@ protected String readString(int length, int offset) throws IOException {
         return new String(data);
     }
 
+    private void checkLength(int length) throws IOException {
+        if (length < 0) {
+            throw new IOException("Invalid negative J3O length/count: " + length);
+        }
+    }
+
+    private int readUnsignedByte(InputStream input, String label) throws IOException {
+        int value = input.read();
+        if (value < 0) {
+            throw new EOFException("Unexpected end of J3O while reading " + label);
+        }
+        return value;
+    }
+
     public Savable readObject(int id) {
 
         if(contentTable.get(id) != null) {
             return contentTable.get(id);
         }
 
         try {
-            int loc = locationTable.get(id);
+            Integer objectLocation = locationTable.get(id);
+            if (objectLocation == null) {
+                throw new IOException("Missing J3O object location for id: " + id);
+            }
+            int loc = objectLocation;
+            if (loc < 0 || loc >= dataArray.length) {
+                throw new IOException("J3O object location outside payload: " + loc);
+            }
 
             String alias = readString(aliasWidth, loc);
             loc+=aliasWidth;
@@ -326,10 +390,16 @@ public Savable readObject(int id) {
                 return null;
             }
 
+            if (loc + 4 > dataArray.length) {
+                throw new IOException("Truncated J3O object length at payload offset: " + loc);
+            }
             int dataLength = ByteUtils.convertIntFromBytes(dataArray, loc);
             loc+=4;
+            if (dataLength < 0 || loc + dataLength < loc || loc + dataLength > dataArray.length) {
+                throw new IOException("Invalid J3O object data length: " + dataLength);
+            }
 
-            Savable  out = SavableClassUtil.fromName(bco.className);
+            Savable  out = SavableClassUtil.fromName(bco.className, classFilter);
 
             BinaryInputCapsule cap = new BinaryInputCapsule(this, out, bco);
             cap.setContent(dataArray, loc, loc+dataLength);
@@ -348,4 +418,4 @@ public Savable readObject(int id) {
             return null;
         }
     }
-}
+}