dengzhhu653 commented on code in PR #6726:
URL: https://github.com/apache/hive/pull/6726#discussion_r3964252909
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/ppr/PartitionExpressionForMetastore.java:
##########
@@ -111,21 +115,64 @@ private ExprNodeDesc deserializeExpr(byte[] exprBytes)
throws MetaException {
try {
expr =
SerializationUtilities.deserializeObjectWithTypeInformation(exprBytes, true);
} catch (Exception ex) {
- LOG.error("Failed to deserialize the expression, fall back to
deserializeObjectFromKryo", ex);
+ LOG.error("Failed to deserialize the expression, fall back to
deserializeUntrustedObjectFromKryo", ex);
try {
- expr = SerializationUtilities.deserializeObjectFromKryo(exprBytes,
ExprNodeGenericFuncDesc.class);
+ // The fallback must use the same untrusted-payload restrictions as
the primary path: these bytes come straight
+ // from a Thrift client.
+ expr =
SerializationUtilities.deserializeUntrustedObjectFromKryo(exprBytes,
ExprNodeGenericFuncDesc.class);
} catch (Exception e) {
LOG.error("Failed to deserialize the expression", e);
throw new
MetaException("SerializationUtilities#deserializeObjectWithTypeInformation: " +
ex.getMessage() +
- ", SerializationUtilities#deserializeObjectFromKryo: " +
e.getMessage());
+ ", SerializationUtilities#deserializeUntrustedObjectFromKryo: " +
e.getMessage());
}
}
if (expr == null) {
throw new MetaException("Failed to deserialize expression - ExprNodeDesc
not present");
}
+ validateDeserializedExpr(expr);
return expr;
}
+ /**
+ * Rejects client-supplied expression graphs that would execute arbitrary
code when the metastore stringifies or
+ * evaluates them. The Kryo-level class allowlist already blocks
reflect/reflect2/java_method/in_file; a
+ * {@link GenericUDFBridge} instance is legitimate (it wraps builtin
old-style UDFs like year()), but it instantiates
+ * whatever class name its {@code udfClassName} field carries, so that name
must resolve to a real {@link UDF}.
+ */
+ private void validateDeserializedExpr(ExprNodeDesc expr) throws
MetaException {
+ if (expr instanceof ExprNodeGenericFuncDesc exprNodeGenericFuncDesc) {
+ validateDeserializedExprNodeGenericFuncDesc(exprNodeGenericFuncDesc);
+ }
+ if (expr.getChildren() != null) {
+ for (ExprNodeDesc child : expr.getChildren()) {
+ validateDeserializedExpr(child);
+ }
+ }
+ }
+
+ private void
validateDeserializedExprNodeGenericFuncDesc(ExprNodeGenericFuncDesc expr)
throws MetaException {
+ GenericUDF genericUDF = expr.getGenericUDF();
+ if (genericUDF instanceof GenericUDFBridge genericUDFBridge) {
+ String udfClassName = genericUDFBridge.getUdfClassName();
+ Class<?> udfClass;
Review Comment:
How about `genericUDF.getUdfClass()` to get the `udfClass`, also there
might be a problem upon class initialization.
--
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]
