uschindler commented on code in PR #16138:
URL: https://github.com/apache/lucene/pull/16138#discussion_r3328572079
##########
lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/SerializableObject.java:
##########
@@ -216,7 +216,13 @@ static Class<?> readClass(final InputStream inputStream)
return StandardObjects.CODE_REGISTRY.get(index);
} else {
String className = readString(inputStream);
- return Class.forName(className);
+ // Load without initializing and confirm the named class is actually a
SerializableObject
+ // before it can be instantiated, so a crafted stream cannot load
arbitrary classes.
+ Class<?> clazz = Class.forName(className, false,
SerializableObject.class.getClassLoader());
Review Comment:
Instead of the antique clasloader magic, the preferable thing is to load the
class with a static final `MethodHandles#lookup()` which is created for this
class. This would make the whole thing behave like an `LDC` instruction, that
does NOT load the class and initializes it. Instead it looks up the class and
makes sure it is accessible from this class.
So basically:
```java
private static final LOOKUP = MethodHandles.lookup();
// later in code here:
Class<?> clazz = LOOKUP.findClass(className);
```
It also does not return null and works like an `LDC` instruction.
Anyways, I'd like to rewrite the rest of the code to use MethodHandles, too.
But that's outside of the scope of this PR.
##########
lucene/spatial3d/src/java/org/apache/lucene/spatial3d/geom/SerializableObject.java:
##########
@@ -216,7 +216,13 @@ static Class<?> readClass(final InputStream inputStream)
return StandardObjects.CODE_REGISTRY.get(index);
} else {
String className = readString(inputStream);
- return Class.forName(className);
+ // Load without initializing and confirm the named class is actually a
SerializableObject
+ // before it can be instantiated, so a crafted stream cannot load
arbitrary classes.
+ Class<?> clazz = Class.forName(className, false,
SerializableObject.class.getClassLoader());
Review Comment:
The pro on this approach is that it also makes sure that we can access the
class (it is visible to us, so invoking ctor works and so on).
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
