Copilot commented on code in PR #7973:
URL: https://github.com/apache/incubator-seata/pull/7973#discussion_r3009264128
##########
json-common/src/main/java/org/apache/seata/common/json/impl/JacksonJsonSerializer.java:
##########
@@ -152,8 +166,42 @@ public <T> T parseObject(String json, Class<T> type,
boolean ignoreAutoType) {
} else {
return objectMapperWithAutoType.readValue(json, type);
}
+ } catch (SecurityException e) {
+ throw e;
} catch (IOException e) {
+ rethrowIfSecurityException(e);
throw new JsonParseException("Jackson deserialize error", e);
}
}
+
+ private static void rethrowIfSecurityException(Throwable e) {
+ Throwable cause = e.getCause();
+ while (cause != null) {
+ if (cause instanceof SecurityException) {
+ throw (SecurityException) cause;
+ }
+ cause = cause.getCause();
+ }
+ }
+
+ private static class AllowlistTypeValidator extends
PolymorphicTypeValidator.Base {
+ private static final long serialVersionUID = 1L;
+
+ @Override
+ public Validity validateBaseType(MapperConfig<?> config, JavaType
baseType) {
+ return Validity.INDETERMINATE;
+ }
+
+ @Override
+ public Validity validateSubClassName(MapperConfig<?> config, JavaType
baseType, String subClassName) {
+ // Throws SecurityException if not allowed
+ JsonAllowlistManager.getInstance().checkClass(subClassName);
+ return Validity.ALLOWED;
+ }
+
+ @Override
+ public Validity validateSubType(MapperConfig<?> config, JavaType
baseType, JavaType subType) {
Review Comment:
AllowlistTypeValidator.validateSubType currently returns Validity.ALLOWED
without checking the resolved subtype against JsonAllowlistManager. Jackson may
invoke validateSubType (not just validateSubClassName), which would bypass the
allowlist in those code paths. Add the same check in validateSubType (e.g.,
checkClass(subType.getRawClass().getName())) so the allowlist is enforced
regardless of which validator callback Jackson uses.
```suggestion
public Validity validateSubType(MapperConfig<?> config, JavaType
baseType, JavaType subType) {
// Throws SecurityException if not allowed
JsonAllowlistManager.getInstance().checkClass(subType.getRawClass().getName());
```
##########
json-common/src/main/java/org/apache/seata/common/json/impl/FastjsonJsonSerializer.java:
##########
@@ -56,6 +58,20 @@ public class FastjsonJsonSerializer implements
JsonSerializer {
private static final Feature[] READER_FEATURES_IGNORE_AUTO_TYPE =
new Feature[] {Feature.IgnoreAutoType, Feature.OrderedField};
+ private static final ParserConfig ALLOWLIST_PARSER_CONFIG = new
ParserConfig();
+
+ static {
+ ALLOWLIST_PARSER_CONFIG.setAutoTypeSupport(true);
+ ALLOWLIST_PARSER_CONFIG.addAutoTypeCheckHandler((typeName,
expectClass, features) -> {
+ JsonAllowlistManager.getInstance().checkClass(typeName);
+ try {
+ return Class.forName(typeName);
+ } catch (ClassNotFoundException e) {
+ return null;
+ }
Review Comment:
The Fastjson AutoTypeCheckHandler uses Class.forName(typeName), which (1)
ignores the thread context classloader and can fail to load user classes in
typical container/classloader setups, and (2) may bypass Fastjson's own
auto-type checks/blacklists if a non-null Class is returned. Prefer only
enforcing JsonAllowlistManager.checkClass(typeName) and returning null (letting
Fastjson load the class via its normal mechanism/classloader), or explicitly
load via an appropriate classloader (e.g., TCCL / expectClass's loader) without
bypassing additional Fastjson safeguards.
```suggestion
// Enforce allowlist, then let Fastjson perform class loading
via its own mechanisms.
JsonAllowlistManager.getInstance().checkClass(typeName);
return null;
```
--
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]
