hazelcast-0001: SerializationUtil.getInterfaces diamond O(2^D) 52428x; count 663→664
This commit is contained in:
parent
8c4b291108
commit
4a6878e383
1 changed files with 76 additions and 0 deletions
|
|
@ -0,0 +1,76 @@
|
|||
# UNDF: (pending)
|
||||
# hazelcast-0001: SerializationUtil.getInterfaces — O(2^D) diamond re-traversal; Collections.addAll() return value ignored
|
||||
|
||||
## CWE-407 — Algorithmic Complexity: O(2^D) recursive diamond re-traversal in interface hierarchy collection for serializer lookup
|
||||
|
||||
| Field | Value |
|
||||
|--------------|-------|
|
||||
| ID | hazelcast-0001 |
|
||||
| Severity | HIGH |
|
||||
| Ecosystem | hazelcast |
|
||||
| Package | hazelcast |
|
||||
| File | `hazelcast/src/main/java/com/hazelcast/internal/serialization/impl/SerializationUtil.java` |
|
||||
| Lines | 150–158 |
|
||||
| Complexity | O(2^D) on diamond interface hierarchies |
|
||||
| Hot path | Called per serialization of a type not yet seen by the serialization service (first-time serializer lookup) |
|
||||
|
||||
## Defect
|
||||
|
||||
`SerializationUtil.getInterfaces` recursively collects all interfaces of a class into a `Set<Class<?>>`.
|
||||
Although it uses a Set, the recursion guard is MISSING — `Collections.addAll(interfaces, classes)` fills
|
||||
the set but the `for` loop then recurses into every interface unconditionally, regardless of whether
|
||||
each was already present in the set:
|
||||
|
||||
```java
|
||||
// SerializationUtil.java:150-158 (DEFECT)
|
||||
static void getInterfaces(Class<?> clazz, Set<Class<?>> interfaces) {
|
||||
final var classes = clazz.getInterfaces();
|
||||
if (classes.length > 0) {
|
||||
Collections.addAll(interfaces, classes); // adds to set (ignored return vals)
|
||||
for (Class<?> cl : classes) {
|
||||
getInterfaces(cl, interfaces); // DEFECT: recurses unconditionally
|
||||
} // ignores whether cl was already present
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
On a diamond hierarchy (interface I1 extends Base; interface I2 extends Base; class C implements I1, I2):
|
||||
- `getInterfaces(C)`:
|
||||
- `Collections.addAll(interfaces, [I1, I2])` → both added
|
||||
- recurse into I1: `getInterfaces(I1)`:
|
||||
- adds Base to set, recurses into Base
|
||||
- recurse into I2: `getInterfaces(I2)`:
|
||||
- `Collections.addAll(interfaces, [Base])` → Base already present, add() returns false (IGNORED)
|
||||
- STILL recurses into Base → unnecessary re-traversal
|
||||
|
||||
At diamond depth D, Base is visited 2^D times.
|
||||
|
||||
Called from `AbstractSerializationService.lookupCustomSerializer()` (line 663, and again once per
|
||||
superclass in the superclass chain at line 669), which is invoked on every first serialization
|
||||
of a previously-unseen class type.
|
||||
|
||||
## Fix
|
||||
|
||||
Replace `Collections.addAll + unconditional loop` with a per-element `interfaces.add()` guard:
|
||||
|
||||
```java
|
||||
// AFTER — O(N+E) where N=interfaces, E=inheritance edges
|
||||
static void getInterfaces(Class<?> clazz, Set<Class<?>> interfaces) {
|
||||
for (Class<?> cl : clazz.getInterfaces()) {
|
||||
if (interfaces.add(cl)) { // add returns false if already present → skip
|
||||
getInterfaces(cl, interfaces); // only recurse if newly added
|
||||
}
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
## Speedup
|
||||
|
||||
| Diamond depth (D) | Before (visits) | After (visits) | Speedup |
|
||||
|------------------|----------------|----------------|---------|
|
||||
| 5 | 31 | 5 | 6× |
|
||||
| 10 | 1,023 | 10 | 102× |
|
||||
| 15 | 32,767 | 15 | 2,184× |
|
||||
| 20 | 1,048,575 | 20 | 52,428× |
|
||||
|
||||
Growth before: O(2^D). Growth after: O(D).
|
||||
Loading…
Add table
Add a link
Reference in a new issue