slfan1989 commented on code in PR #8689:
URL: https://github.com/apache/hadoop/pull/8689#discussion_r3888969377
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/container/ResourceMappings.java:
##########
@@ -89,9 +91,24 @@ public void updateAssignedResources(List<Serializable> list)
{
public static AssignedResources fromBytes(byte[] bytes)
throws IOException {
final List<Serializable> resources;
- try {
- resources = SerializationUtils.deserialize(bytes);
- } catch (SerializationException e) {
+ // The bytes come from the NM recovery state store and are read back
+ // during container recovery on restart. Deserialize through a
+ // ValidatingObjectInputStream so a tampered record cannot instantiate
+ // arbitrary serializable classes on the NodeManager classpath. The
+ // allowed graph is the assigned-resource value objects the resource
+ // plugins store (device / NUMA descriptors, plain strings) plus the
+ // collection types that wrap them.
+ try (ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
+ ValidatingObjectInputStream ois =
+ new ValidatingObjectInputStream(bais)) {
+ ois.accept(
+ "org.apache.hadoop.yarn.server.nodemanager.*",
+ "org.apache.hadoop.thirdparty.com.google.common.collect.*",
+ "java.util.*",
+ "java.lang.*",
+ "[Ljava.lang.Object;");
Review Comment:
The remaining package wildcard still weakens the stated `known types`
boundary.
For example, a shaded-Guava `ImmutableList` and its serialization proxy
match this pattern and can be accepted even though they are not
assigned-resource value types. Likewise, any serializable class in this package
can be instantiated as an element of an allowed `ArrayList` before the
top-level `instanceof List` check.
Could we enumerate the exact `ImmutableMap` serialization proxy types
required by the current and supported upgrade paths, and cover those paths with
compatibility fixtures? If those internal names cannot be made sufficiently
stable, a versioned non-Java-serialization format may be safer than retaining a
package-wide wildcard.
--
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]
