martin-g commented on code in PR #2934:
URL: https://github.com/apache/avro/pull/2934#discussion_r1639482701


##########
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##########
@@ -24,12 +24,25 @@
 import org.apache.avro.io.ResolvingDecoder;
 import org.apache.avro.util.ClassUtils;
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
 /**
  * {@link org.apache.avro.io.DatumReader DatumReader} for generated Java
  * classes.
  */
 public class SpecificDatumReader<T> extends GenericDatumReader<T> {
+
+  public static final String[] SERIALIZABLE_PACKAGES;
+
+  static {
+    SERIALIZABLE_PACKAGES = 
System.getProperty("org.apache.avro.SERIALIZABLE_PACKAGES",
+        
"java.lang,java.math,java.io,java.net,org.apache.avro.reflect").split(",");
+  }
+
+  private List<String> trustedPackages = new ArrayList<>();

Review Comment:
   ```suggestion
     private final List<String> trustedPackages = new ArrayList<>();
   ```



##########
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##########
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
     if (name == null)
       return null;
     try {
-      return ClassUtils.forName(getData().getClassLoader(), name);
+      Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+      checkSecurity(clazz);
+      return clazz;
     } catch (ClassNotFoundException e) {
       throw new AvroRuntimeException(e);
     }
   }
 
+  private boolean trustAllPackages() {
+    return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));

Review Comment:
   ```suggestion
       return (trustedPackages.size() == 1 && 
"*".equals(trustedPackages.get(0)));
   ```
   



##########
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##########
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
     if (name == null)
       return null;
     try {
-      return ClassUtils.forName(getData().getClassLoader(), name);
+      Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+      checkSecurity(clazz);
+      return clazz;
     } catch (ClassNotFoundException e) {
       throw new AvroRuntimeException(e);
     }
   }
 
+  private boolean trustAllPackages() {
+    return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));
+  }
+
+  private void checkSecurity(Class clazz) throws ClassNotFoundException {
+    if (trustAllPackages() || clazz.isPrimitive()) {
+      return;
+    }
+
+    boolean found = false;
+    Package thePackage = clazz.getPackage();
+    if (thePackage != null) {
+      for (String trustedPackage : getTrustedPackages()) {
+        if (thePackage.getName().equals(trustedPackage) || 
thePackage.getName().startsWith(trustedPackage + ".")) {
+          found = true;
+          break;
+        }
+      }
+      if (!found) {
+        throw new ClassNotFoundException("Forbidden " + clazz
+            + "! This class is not trusted to be included in Avro schema using 
java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property 
with the packages you trust.");
+      }
+    }
+  }
+
+  public List<String> getTrustedPackages() {

Review Comment:
   This method should be `final` or it should be used at 
https://github.com/apache/avro/pull/2934/files#diff-95af304493048b22c3dda8fbfc7efb0e0c1c5e876f0234c2b8d0635bb2bcb494R127



##########
lang/java/avro/src/main/java/org/apache/avro/specific/SpecificDatumReader.java:
##########
@@ -101,12 +115,43 @@ private Class getPropAsClass(Schema schema, String prop) {
     if (name == null)
       return null;
     try {
-      return ClassUtils.forName(getData().getClassLoader(), name);
+      Class clazz = ClassUtils.forName(getData().getClassLoader(), name);
+      checkSecurity(clazz);
+      return clazz;
     } catch (ClassNotFoundException e) {
       throw new AvroRuntimeException(e);
     }
   }
 
+  private boolean trustAllPackages() {
+    return (trustedPackages.size() == 1 && trustedPackages.get(0).equals("*"));
+  }
+
+  private void checkSecurity(Class clazz) throws ClassNotFoundException {
+    if (trustAllPackages() || clazz.isPrimitive()) {
+      return;
+    }
+
+    boolean found = false;
+    Package thePackage = clazz.getPackage();
+    if (thePackage != null) {
+      for (String trustedPackage : getTrustedPackages()) {
+        if (thePackage.getName().equals(trustedPackage) || 
thePackage.getName().startsWith(trustedPackage + ".")) {
+          found = true;
+          break;
+        }
+      }
+      if (!found) {
+        throw new ClassNotFoundException("Forbidden " + clazz
+            + "! This class is not trusted to be included in Avro schema using 
java-class. Please set org.apache.avro.SERIALIZABLE_PACKAGES system property 
with the packages you trust.");

Review Comment:
   IMO the exception should mention the problematic class name. Currently it is 
hard for the developer to realize which class causes the problem.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@avro.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to