diff --git a/changes/en-us/2.x.md b/changes/en-us/2.x.md index 9cdb1687f5d..63d3c5824b6 100644 --- a/changes/en-us/2.x.md +++ b/changes/en-us/2.x.md @@ -87,6 +87,7 @@ Add changes here for all PR submitted to the 2.x branch. - [[#8109](https://github.com/apache/incubator-seata/pull/8109)] reduce npmjs dependencies in saga module - [[#8112](https://github.com/apache/incubator-seata/pull/8112)] optimize seata-server test performance - [[#8116](https://github.com/apache/incubator-seata/pull/8116)] fix incompatible dependencies +- [[#8120](https://github.com/apache/incubator-seata/pull/8120)] limit allowlist cache growth ### security: diff --git a/changes/zh-cn/2.x.md b/changes/zh-cn/2.x.md index 285ccfc0e02..74c3530b4a9 100644 --- a/changes/zh-cn/2.x.md +++ b/changes/zh-cn/2.x.md @@ -89,6 +89,7 @@ - [[#8109](https://github.com/apache/incubator-seata/pull/8109)] 精简 saga 模块 npmjs 依赖 - [[#8112](https://github.com/apache/incubator-seata/pull/8112)] 优化 seata-server 测试性能 - [[#8116](https://github.com/apache/incubator-seata/pull/8116)] 修复不兼容的依赖 +- [[#8120](https://github.com/apache/incubator-seata/pull/8120)] 限制了json-common白名单缓存 ### security: diff --git a/json-common/json-common-core/src/main/java/org/apache/seata/common/json/JsonAllowlistManager.java b/json-common/json-common-core/src/main/java/org/apache/seata/common/json/JsonAllowlistManager.java index f40264d88b2..09078b47c05 100644 --- a/json-common/json-common-core/src/main/java/org/apache/seata/common/json/JsonAllowlistManager.java +++ b/json-common/json-common-core/src/main/java/org/apache/seata/common/json/JsonAllowlistManager.java @@ -27,6 +27,12 @@ public class JsonAllowlistManager { private static final JsonAllowlistManager INSTANCE = new JsonAllowlistManager(); + private static final int MAX_CLASS_NAME_LENGTH = 1024; + + private static final int MAX_REPORTED_CLASS_NAME_LENGTH = 256; + + private static final int MAX_CACHE_SIZE = 4096; + /** * Built-in exact match allowlist */ @@ -107,10 +113,18 @@ public void addUserPrefix(String prefix) { * Check if a class is allowed for deserialization */ public boolean isAllowed(String className) { - if (className == null) { + if (className == null || className.length() > MAX_CLASS_NAME_LENGTH) { return false; } - return cache.computeIfAbsent(className, this::doCheck); + if (cache.get(className) != null) { + return true; + } + + boolean allowed = doCheck(className); + if (allowed) { + cacheAllowedClass(className); + } + return allowed; } /** @@ -118,8 +132,8 @@ public boolean isAllowed(String className) { */ public void checkClass(String className) { if (!isAllowed(className)) { - throw new SecurityException("Class not in JSON deserialization allowlist: " + className - + ". Please add it to seata.json.allowlist configuration."); + throw new SecurityException("Class not in JSON deserialization allowlist: " + + formatClassNameForMessage(className) + ". Please add it to seata.json.allowlist configuration."); } } @@ -143,6 +157,24 @@ private boolean doCheck(String className) { return componentClassName != null && isExactOrPrefixAllowed(componentClassName); } + private void cacheAllowedClass(String className) { + synchronized (cache) { + if (cache.size() < MAX_CACHE_SIZE) { + cache.put(className, Boolean.TRUE); + } + } + } + + private String formatClassNameForMessage(String className) { + if (className == null || className.length() <= MAX_REPORTED_CLASS_NAME_LENGTH) { + return String.valueOf(className); + } + return className.substring(0, MAX_REPORTED_CLASS_NAME_LENGTH) + + "...(truncated, length=" + + className.length() + + ")"; + } + /** * Check if className is a multi-dimensional primitive array descriptor (e.g. "[[I", "[[Z"). * Single-dimensional primitive arrays (e.g. "[I") are already in the builtin allowlist. diff --git a/json-common/json-common-core/src/test/java/org/apache/seata/common/json/JsonAllowlistManagerTest.java b/json-common/json-common-core/src/test/java/org/apache/seata/common/json/JsonAllowlistManagerTest.java index c33425308ba..3b0fe5c467c 100644 --- a/json-common/json-common-core/src/test/java/org/apache/seata/common/json/JsonAllowlistManagerTest.java +++ b/json-common/json-common-core/src/test/java/org/apache/seata/common/json/JsonAllowlistManagerTest.java @@ -19,6 +19,9 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import java.lang.reflect.Field; +import java.util.Map; + import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -268,6 +271,18 @@ public void testCheckClass_notAllowed() { .hasMessageContaining("seata.json.allowlist"); } + @Test + public void testCheckClass_overlongNameIsTruncatedInMessage() { + JsonAllowlistManager manager = JsonAllowlistManager.getInstance(); + String className = buildClassName(1100); + + assertThatThrownBy(() -> manager.checkClass(className)) + .isInstanceOf(SecurityException.class) + .hasMessageContaining("not in JSON deserialization allowlist") + .hasMessageContaining("truncated, length=1100") + .hasMessageNotContaining(className); + } + @Test public void testCheckClass_userAllowed() { JsonAllowlistManager manager = JsonAllowlistManager.getInstance(); @@ -318,6 +333,7 @@ public void testSingleton() { @Test public void testCacheWorks() { JsonAllowlistManager manager = JsonAllowlistManager.getInstance(); + manager.clearUserAllowlist(); boolean result1 = manager.isAllowed("java.lang.String"); @@ -325,6 +341,43 @@ public void testCacheWorks() { assertThat(result1).isTrue(); assertThat(result2).isTrue(); + assertThat(cacheSize(manager)).isEqualTo(1); + } + + @Test + public void testRejectedClassesAreNotCached() { + JsonAllowlistManager manager = JsonAllowlistManager.getInstance(); + manager.clearUserAllowlist(); + + for (int i = 0; i < 100; i++) { + assertThat(manager.isAllowed("com.malicious.EvilClass" + i)).isFalse(); + } + + assertThat(cacheSize(manager)).isZero(); + } + + @Test + public void testOverlongClassNameIsRejectedAndNotCached() { + JsonAllowlistManager manager = JsonAllowlistManager.getInstance(); + manager.clearUserAllowlist(); + String className = buildClassName(1100); + + assertThat(manager.isAllowed(className)).isFalse(); + + assertThat(cacheSize(manager)).isZero(); + } + + @Test + public void testAllowedClassCacheIsBounded() { + JsonAllowlistManager manager = JsonAllowlistManager.getInstance(); + manager.clearUserAllowlist(); + + for (int i = 0; i < 5000; i++) { + assertThat(manager.isAllowed("org.apache.seata.generated.AllowedClass" + i)) + .isTrue(); + } + + assertThat(cacheSize(manager)).isEqualTo(4096); } @Test @@ -337,4 +390,26 @@ public void testCacheClearedOnLoadUserAllowlist() { assertThat(manager.isAllowed("com.example.CacheTest")).isTrue(); } + + @SuppressWarnings("unchecked") + private static int cacheSize(JsonAllowlistManager manager) { + try { + Field cacheField = JsonAllowlistManager.class.getDeclaredField("cache"); + cacheField.setAccessible(true); + Map cache = (Map) cacheField.get(manager); + return cache.size(); + } catch (ReflectiveOperationException e) { + throw new AssertionError("Failed to read JsonAllowlistManager cache", e); + } + } + + private static String buildClassName(int length) { + String prefix = "com.example."; + StringBuilder builder = new StringBuilder(length); + builder.append(prefix); + while (builder.length() < length) { + builder.append('A'); + } + return builder.toString(); + } }